Consumer flow strategy (take two)

[acogoluegnes]

Rough draft.

[acogoluegnes]

@henry701 I made some progress. WRT the tasks I mentioned:

• I let the implementation classes in the interface file, they are accessible with static helpers (same pattern as BackOffDelayPolicy)
• the MessageProcessedCallback accepts now a MessageHandler.Context (it was a LongConsumer with the offset before). The MessageHandler.Context contains all the necessary information. This is a way to re-use the instances and avoid creating too many objects.
• I did not make Client generic (with the "chunk context" type as the type parameter). It was a big change that would make code awkward in other places. Client is an internal class, the code base can live with this small "flaw".
• I added a MessageIgnoredListener callback for messages that are ignored at the beginning of the first chunk of a subscription.

I did not make the call to processed idempotent. I don't see an efficient way to implement it, so users will have to be careful. It shouldn't be critical for now, we'll see if we get some feedback.

Next steps are Javadoc and documentation.

Let me know what you think.

[henry701]

I'll have a look at the code later, busy week here, unfortunately. But regarding the comment, yeah, seems nice. Do we need chunk context being a type parameter? It makes sense for having a wide variety of strategies, but maybe we can just drop the idea since the implementation has been greatly simplified. Then we don't need to cast in Client and nothing needs to use generics.

[henry701]

Looks superb

[acogoluegnes]

Do we need chunk context being a type parameter? It makes sense for having a wide variety of strategies, but maybe we can just drop the idea since the implementation has been greatly simplified. Then we don't need to cast in Client and nothing needs to use generics.

The chunk context is used only in the ConsumersCoordinator, so we should not hit a cast problem anywhere else in the library. I started to make Client generic and the genericity was more of a burden than a benefit as it needed to be Void everywhere but in ConsumersCoordinator. Client is not a public API, so I'd say it's fine.

Thanks for the review!

[acogoluegnes]

Do we need chunk context being a type parameter? It makes sense for having a wide variety of strategies, but maybe we can just drop the idea since the implementation has been greatly simplified. Then we don't need to cast in Client and nothing needs to use generics.

Sorry for my comment above, I did not get your comment right. You are suggesting to make the "chunk context" the type we need for the consumer flow strategy, that is ConsumerFlowStrategy.MessageProcessedCallback, because we don't need it anywhere else. That's opinionated, but as it's just internal and it's not needed anywhere else, why not?

[henry701]

I'll test this with the use-case of issue #333 later

[henry701]

It worked like a charm in some local ad-hoc tests, never surpassing a certain threshold of messages (and certainly not two chunks at a time), inspecting the map of in-memory messages that I used here.
Also, the API and Javadoc are very clean and clear.
Nice!

[acogoluegnes]

Thanks @henry701 for testing this! Let's merge it.