RandomDestination at first & round-robin

[withccm]

Hello.
Logstash-logback-encoder is woking by primary and secondary.
I need a balanced request to each destination.
@brenuart registered the issue #125

• The first set destination is random.
• If the size is exceeded, round-robin occurs.

I hope to review the code.
Thanks.

[philsttr]

I don't really understand why a size is needed. It seems like you would want to either round robbin, or not. Why would you want to not round robin for a while, and then start round robin?

[withccm]

As you know, if you do round robin, a reconnection occurs.
When reconnecting each time, a big cost was introduced, and the "roundRobinSize" was introduced as a concept of buffer.

[philsttr]

Ohhhh, I thought it would just round-robin the next time a connection was needed (i.e. if a connection was broken). I didn't notice that your change forces a reconnect after every N log messages. Makes sense now. If this is the case, I think it would make more sense to use a time based round-robin, rather than message count based. That way sudden increases of load don't trigger a reconnection storm.

This is kind of similar to the secondaryConnectionTTL already implemented. However, it applies to any connection. So, maybe it would just be connectionTTL.

[philsttr]

I think we need to introduce a DestinationSelectionStrategy concept.

I can see three strategies at the moment:

1. PreferPrimary - the current strategy of always preferring the primary destination if it is available (secondaryConnectionTTL is only applicable for this strategy)
2. RoundRobin - a new strategy round-robin connects to a destination (starting at the first) every time a reconnect occurs
3. Random - randomly selects a destination every time a reconnect occurs

Would those satisfy your use cases?

From the changes in the current PR, it is not obvious (without studying the code pretty hard) which strategy is being used for any given set of configuration values. For example, how does secondaryConnectionTTL come into play with your new strategies? What happens when it is set? So, I think a first-class DestinationSelectionStrategy concept is needed in order to make it very explicit what strategy is being used.

[brenuart]

+1
That would be very nice indeed.

[withccm]

@philsttr
I will modify it using the concept of the strategy you have told me.

[withccm]

@philsttr
Hello. I have developed three strategies. Please check again.

[philsttr]

Hey @withccm,

I've refactored your code a bit to extract a strategy interface, with three implementations. This pulls the logic out the appender, and allows people to develop custom strategies.

I've committed it to the destination_connection_strategy branch. I still have more testing and documentation to do, but this will provide a preview of how I'm thinking this should work.

[withccm]

@philsttr, Thank you for your positive review.
I have reviewed the code you have been working on.
I would be willing to help if I needed anything.

[philsttr]

4.10 released with this change

[withccm]

Thanks @philsttr