Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create well-known properties under quickfixj.server.config #96

Open
ggershaw opened this issue Dec 24, 2022 · 6 comments
Open

Create well-known properties under quickfixj.server.config #96

ggershaw opened this issue Dec 24, 2022 · 6 comments

Comments

@ggershaw
Copy link

ggershaw commented Dec 24, 2022 •
edited
Loading

Currently, you need to provide the a string which has all default and session level settings in the quickfixj.server.configString property. This doesn't allow you to leverage boot's ability to override yaml properties and violates DRY.

You would need to provide the same value with slight differences for each env (dev, uat, prod)

In the below proposed approach, DRY is respected and we leverage boot's ability to override the sessions property.

The proposed properties are under
'''
quickfixj:
server:
config:
''''

Proposed properties are:

  1. default property which is a string. The dev would put all the default settings here in 1 single string. This property should be populated in application.yaml and should not be overridden in application-uat.yml etc to respect DRY
  2. sessions property which is a list of the individual session level settings. This property would be overridden in application-uat.yml etc. Session level sessions tend to change for each env so this doesn't violate DRY.
    -- create well known properties for the typical session level quickfixj properties
    -- create a catch all map property named additionalSessionSettings for venue specific settings that we just add to the session and assume the dev knows what they are doing

Proposed example
'''
quickfixj:
server:
config:
default: defaultSettings
sessions:
-
name: -- used in code to identify the session's settings
beginString: FIXT.1.1
targetCompId: Venue1
socketConnectHost: someip
socketConnectPort: 9876
additionalSessionSettings:
setting1: value
setting2: value2

        -
            name: venue2
            beginString: FIXT.1.1
            targetCompId: VenueCompId
            socketConnectHost: someip
            socketConnectPort: 9876


---------------------------------
Additionally,

Initiator or Acceptor is determined by 
quickfixj:
  client:
vs.
quickfixj:
  server:

Don't need it in the configString. Violates DRY and could cause bug/confusion if they have diff values.
    ConnectionType=initiator

-----------------------------

Finally, I believe quickfixjserver property should be renamed quickfixj unless this property is only for the server. I'm not clear on that.

management:
  health:
    quickfixjserver:
      enabled: true
@esanchezros
Copy link
Owner

Hi @ggershaw, thanks for your suggestions. I think they are valid and probably the next logical step in improving the starter.

I myself had this same problem, when loading the settings file was only possible via classpath or absolute path. I thought about creating a replica of all quickfixj config settings as you mentioned and load them into quickfixj but it's a significant piece of work and didn't have time to do it (that's why I added configString). There is also the refresh context event (if using Spring Config server) which would require further changes.

I'll try to find the time over holidays to give it a go. Alternatively, feel free to send a PR. All help is welcome.

Thanks

@ggershaw
Copy link
Author

ggershaw commented Dec 25, 2022 via email

@esanchezros
Copy link
Owner

Hi @ggershaw

Have you made a start on this? I may have some capacity soon to get on with this feature if you haven't started yet.

Thanks

@ggershaw
Copy link
Author

Hey @esanchezros ,

I think we should test the performance capability of this solution before you put more effort into it. Let me re-read about spring eventing. I think their might be aync event, but still not sure it's built to handle market data.

If I can't assess from the documentation, the branch I posted on the qfix group will send a bunch of MDIR messages from the server in response to an MDR, which are running in separate processes. I could add code to the FromApp call In the client side to look for delays in the messages.

@esanchezros
Copy link
Owner

Hi @ggershaw,

As I said on that post, Spring Eventing is there as a default implementation and should not be used for high volumes like Market Data. Also, using Async will not guarantee message ordering. There is no issue with performance as far as I know, remember this starter is just a wrapper around quickfixj.

Why don't you provide your own quickfix.Applicationimplemtation? The spring boot starter allows you to override it just by providing a bean of that type. We have services providing market data at a very high throughput.

In this example you can see how to override the default implementation of quickfix.Application.
https://github.com/esanchezros/quickfixj-spring-boot-starter-examples/blob/master/simple-server/src/main/java/io/allune/quickfixj/spring/boot/starter/examples/server/ServerApplicationAdapter.java.

@ggershaw
Copy link
Author

Got it. Will adjust

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@esanchezros @ggershaw