[Broker] Skip response when producerFuture is already complete

[michaeljmarshall]

Motivation

This PR fixes an issue introduced in branch-2.7 by one of my commits. @codelipenghui has some context in this PR #13245 and proposes the opposite solution.

The behavior here aligns with d12486b. Because branch-2.7 does not have this commit, one of my tests in the cherry picked commits is failing.

I think we need to update branch-2.7 to be like master and to only respond if the future is completed by this exception. From my perspective, the contract in the class is that if a thread completes the future, it should also send (or intentionally not send) a response. In the failing test case, the producer future is completed by a CloseProducer command.

Alternatively, we could update the test if we don't want to change this client behavior.

Modifications

• If the producer fails to get created, only send a response to the client if the producerFuture is not already completed.

Verifying this change

I verified this test by running the ServerCnxTest locally with the proposed change. It passed. This change is also already i master.

Does this pull request potentially affect one of the following parts:

This change affects the binary protocol because it skips sending a response to the client. However, it only skips it when the future is already completed, and the completion implies that the ServerCnx has already handled the request.

Documentation

• no-need-doc

[michaeljmarshall]

I talked this through with @codelipenghui over slack, and we agree that we should not change the behavior in branch 2.7. He committed the test fix here: e99777a#diff-b73845cd7706e03b89122a21b3e60c36c95ce6a0c7dd4574f8eda3fc67dd02b1R875-R878.