Infinispan MessageStore Implementation Question

[stephennimmo]

I have started to implement a new MessageStore implementation which uses the in-memory data grid, Infinispan (https://infinispan.org). While I have a good start, I had some questions.

1. The implementation would require the inclusion of the Infinispan Hot Rod Client jar. If I placed the implementation in the same project location as the other implementations (quickfixj-core/src/java/quickfix), then the client jar would be referenced with the core jar and this might not be acceptable.
2. Part of the tests for this requires a running Infinispan cluster. For this, I would prefer to use testcontainers (https://www.testcontainers.org). Before I go too far down this road, I wanted to get some of the core maintainers opinion about introducing testcontainers to the test framework. I think it might be a great addition as we could use the testcontainers to do many more integration tests, such as JDBC backed implementation running against database such as Postgres.

As an alternative, I could create a new separate module in the project (quickfixj-infinispan) and create all the implementation/tests there OR I could just create a whole separate project externally.

Here's the branch for the implementation so far: https://github.com/stephennimmo/quickfixj/tree/infinispan-implementation

Classes are: InfinispanStoreFactory, InfinispanStore

[QwertGold]

The invariant of the fix protocol is that: The sender needs to persist the message before sending.! This is because the sender of messages 'own' the persistence in case the receiver crashes.
I have seen several implementers delegate to Hazelcast or other asynchronous mechanism like Infinispan, technically this is not allowed as it breaks the resending invariant - Messages must be persisted before sending!! Failure is so rare that you can probable get away with an incorrect implementation for years without issue, but if you crash, and cannot resend a message (which was lost on the wire), you are in an unrecoverable state (sequence number too low) and the only options is to tell the acceptor to increment the sequence number and ignore the lost message.

The important requirement of the fix protocol, is that it is the senders responsibility to create persistence, which means EVERY message MUST be persisted before sending, if this invariant is broken, problems can occurs!

[QwertGold]

Today you can only send outgoing messages one at a time with QuickfixJ, this is a recipe for bad performance, and we should probably try find better solutions.

[chrjohn]

but if you crash, and cannot resend a message (which was lost on the wire), you are in an unrecoverable state (sequence number too low)

It's not going to help in every scenario but you can also resend Heartbeats when something goes completely wrong:

quickfixj/quickfixj-core/src/main/java/quickfix/Session.java

Lines 2292 to 2296 in 2f4fda1

	try {
	state.get(beginSeqNo, endSeqNo, messages);
	} catch (final IOException e) {
	if (forceResendWhenCorruptedStore) {
	LOG.error("Cannot read messages from stores, resend HeartBeats", e);

[stephennimmo]

With most of the IMDG products (Infinispan, Hazelcast), the cache put is actually representative of not only the receipt of the message from the IMDG, but it also represents that the messages has also been backed up in a secondary node. It can also allow for "write-behind" or even "write-though" which will persist the data somewhere else (disk, DB, etc).

Deferring to a correctly configured IMDG as a persistence mechanism would not create any of the invariant issues as described.

[chrjohn]

Hi @stephennimmo ,

thanks for the idea and sorry for the long delay in answering. I don't have any experience with Inifinispan but the store you created looks good to me.

IMHO it would be good to create a new submodule (or even sub-sub-module, maybe quickfixj-store/quickfixj-infinispan). There is another issue which might be related (#459) since it also deals with moving out some storage code from core to a sub-module.

W.r.t. the tests: I think it would be good to have some kind of integration tests.
Maybe some of the other regular committers also have an opinion on this. @the-thing @philipwhiuk any comments?

Thanks and cheers,
Chris.

[philipwhiuk]

IMHO it would be good to create a new submodule (or even sub-sub-module, maybe quickfixj-store/quickfixj-infinispan).

I agree (not sure about sub-submodule - it's always seemed unnecessary to me)

W.r.t. the tests: I think it would be good to have some kind of integration tests.

Definitely, if you can ensure they're not flaky.

[david-gibbs-ig]

I think that a new capability such as this should be in a new repository and built/published independently. In some ways this is not quite as easy to maintain but avoids gradually increasing the complexity and dependencies of the "core" QFJ project. This helps with conformance to the "stable dependencies principle". This is not to say the project can't be endorsed by the QFJ project and/or be part of the quickfix-j organisation. The partly or fully independent project can also still use the quickfixj-parent as its parent pom if this is helpful.

To use the capability an application need only add the optional dependency.

Because this is a new capability, to aid maintainability it would be better to use a distinct package name. So I think a separate repo is better. Whether a separate organisation or not then QFJ can refer to it in documents, for example in "quickfixj.github.io" GitHub pages.