Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-3689 - process only distinct connectionIds within processDisconnected() #1658

Closed
wants to merge 5 commits into from

Conversation

@rnpridgeon
Copy link
Contributor

commented Jul 24, 2016

@ijuma discovered a possible scenario in which connectionQuotas may be doubly-decremented

This PR intends to remove that possibility by first filtering out any duplicate connectionIds. This way each

@rnpridgeon rnpridgeon changed the title Kafka 3689 - process only distinct connectionIds within processDisconnected() KAFKA-3689 - process only distinct connectionIds within processDisconnected() Jul 24, 2016
// Duplicate connectionId entries could lead to a double decrement of the connection quota which is keyed by host address
val distinct = selector.disconnected.asScala.distinct
if (distinct.size != selector.disconnected.size)
warn("disconnected list has duplicates")

This comment has been minimized.

Copy link
@ijuma

ijuma Jul 25, 2016

Contributor

It may be worth including the duplicates in the warning.

This comment has been minimized.

Copy link
@rnpridgeon

rnpridgeon Jul 25, 2016

Author Contributor

good point, done

@ijuma

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

Thanks for the PR. A few comments:

  1. It would be great to describe a scenario where we could end up with duplicate ids in the disconnected list (a test would be even better).
  2. Depending on the scenario for 1, it may be better for Selector.disconnected to be a Set instead of List.

I'd be interested to hear @junrao's thoughts on this.

@rnpridgeon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2016

Thanks @ijuma

  1. I believe this can happen when quickly killing then restarting clients. This could put connectionID into the disconnected list as failedSends once at the end of a poll(). If the same connectionID is added to the disconnected list again then we could see the same id twice.
  2. True, that is a much better solution, I'll try to get that to you today.

I have requested a sit down with Jun to review my observations on this 1657. I'll update both PRs with my notes on that

@rnpridgeon

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2018

closing as I have not had time to revisit.

@rnpridgeon rnpridgeon closed this May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.