-
Notifications
You must be signed in to change notification settings - Fork 656
[QFJ-968] Allow provision of custom Message Queue implementation #380
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.
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
Conversation
|
This is a very useful change. I want this featue/improvement to be included in the next release version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@korzha thanks for the PR. In general I think this could be useful.
Could you please take a look? About 750 tests are failing.
Could you also please fix the indentation (2 spaces instead of 4).
On another note: did you try using the already implemented watermark-based queue? It propagates the back pressure to the socket and could prevent the OOM. However, given the amount of data you receive it could quickly fill up the socket buffer of the sending side.
|
@wangvic apart from the things that still need to be looked at this PR could probably be part of 3.0.0 or the minor release after the next. |
chrjohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests need fixing. Looks like a general problem around Logon process.
|
@chrjohn Yes, sorry for the noise. And thanks for suggestion about using watermark queue. Unfortunately we need to keep all the messages we receive without blocking upstream. |
614dcd8 to
be776e4
Compare
|
Thanks so far, I will review in due course. Looking good at first sight. |
|
Closing and reopening to trigger new build jobs. |
chrjohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Finally gotten around to merge it. Had to do two minor changes in the tests.
…ckfix-j#380) * [QFJ-968] Allow provision of custom Message Queue implementation Co-authored-by: Andrei Korzhevskii <andrei.korzhevskii@gs.com> Co-authored-by: Christoph John <christoph.john@macd.com>
We have ran into similar issue described here https://www.quickfixj.org/jira/browse/QFJ-968
We have a huge fix message flow and if we restart our service after a while (after fixing some errors),
our resend request becomes really big and takes a few minutes to fulfill.
Live messages go into the SessionState's messageQueue and fill our whole memory (tens of gigs) in just a few minutes very quickly and the process crashes OOM.
My proposal is to allow to provide a custom implementation of the message queue like you do with MessageStore for example. This will let us store have our implementation of the queue to flush queue on disk.
I've added new constructor to Session to keep old api in place.