KAFKA-6101 Reconnecting to broker does not exponentially backoff

[tedyu]

For reconnection, share the nodeState if one exists.
Only update its state and lastConnectAttemptMs.

[tedyu]

retest this please

[ijuma]

Thanks for the PR. Can we please add a test?

[tedyu]

I ran thru NetworkClientTest but the tests didn't exercise the code path I modified.

Suggestion on how to emulate the scenario is appreciated.

[tedyu]

None of the clients tests references ClusterConnectionStates.
I can add a counter which is incremented in the if block below:

    public void connecting(String id, long now) {
        if (nodeState.containsKey(id)) {

Some test can assert that the counter is greater than one.

But ClusterConnectionStates is not public.

[tedyu]

KafkaProducer doesn't expose getter for Sender.
Still looking for way(s) to access ClusterConnectionStates from test.

[soenkeliebau]

Is there a specific reason for adding this in? I can't find any usages of the logger ..

[tedyu]

Ran the following using Java 8 which passed:

./gradlew -PscalaVersion=2.12 -Dtest.single=ResetIntegrationTest streams:test

[soenkeliebau]

Hi @tedyu, you are indeed right, there are no tests covering this code, however I'd argue that there should be. I've created some test cases in a pull request to cover the basic workings of this class as I understand it. I've left out a test for the code you fixed for now, as it'd fail anyway.
I've also created a pull request against your fork with the skeleton of the test class, so you should be able to add your test cases in there and everything should merge later on, regardless of the order it may be applied in.

[tedyu]

Thanks, @soenkeliebau

Please give Sönke credit since the test came from him.

[soenkeliebau]

Hi @tedyu - I may have phrased my last comment badly, the test I referenced does not test proper backoff behavior but only the maximum backoff value, to test your code we need a test case that checks for the backoff to be within the proper range for a few reconnect attempts. I've sent you a new pull request with a proposal of how a test case like that could look.

[tedyu]

retest this please

[ijuma]

@tedyu, can you please rebase against trunk? Thanks.

[tedyu]

Let me open a new PR.