Skip to content

HOTFIX: only try to clear discover-coordinator future upon commit#12259

Merged
ijuma merged 1 commit intoapache:3.1from
guozhangwang:HOTFIX-3-1-poll-network-for-commit-async
Jun 7, 2022
Merged

HOTFIX: only try to clear discover-coordinator future upon commit#12259
ijuma merged 1 commit intoapache:3.1from
guozhangwang:HOTFIX-3-1-poll-network-for-commit-async

Conversation

@guozhangwang
Copy link
Copy Markdown
Contributor

This is a cherrypick commit of 3.1. Another way of fixing KAFKA-13563 other than #11631.

Instead of letting the consumer to always try to discover coordinator in pool with either mode (subscribe / assign), we defer the clearance of discover future upon committing async only. More specifically, under manual assign mode, there are only three places where we need the coordinator:

  • commitAsync (both by the consumer itself or triggered by caller), this is where we want to fix.
  • commitSync, which we already try to re-discovery coordinator.
  • committed (both by the consumer itself based on reset policy, or triggered by caller), which we already try to re-discovery coordinator.

The benefits are that for manual assign mode that does not try to trigger any of the above three, then we never would be discovering coordinator. The original fix in #11631 would let the consumer to discover coordinator even if none of the above operations are required.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Copy Markdown
Member

@showuon showuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks.

@ijuma ijuma merged commit a53b410 into apache:3.1 Jun 7, 2022
philipnee pushed a commit to confluentinc/kafka that referenced this pull request Jun 7, 2022
…ache#12244) (apache#12259)

This is a cherrypick commit of 3.1. Another way of fixing KAFKA-13563 other than apache#11631.

Instead of letting the consumer to always try to discover coordinator in pool with either mode (subscribe / assign), we defer the clearance of discover future upon committing async only. More specifically, under manual assign mode, there are only three places where we need the coordinator:

commitAsync (both by the consumer itself or triggered by caller), this is where we want to fix.
commitSync, which we already try to re-discovery coordinator.
committed (both by the consumer itself based on reset policy, or triggered by caller), which we already try to re-discovery coordinator.
The benefits are that for manual assign mode that does not try to trigger any of the above three, then we never would be discovering coordinator. The original fix in apache#11631 would let the consumer to discover coordinator even if none of the above operations are required.

Reviewers: Luke Chen <showuon@gmail.com>, David Jacot <djacot@confluent.io>
philipnee added a commit to confluentinc/kafka that referenced this pull request Jun 8, 2022
…ache#12244) (apache#12259) (#723)

This is a cherrypick commit of 3.1. Another way of fixing KAFKA-13563 other than apache#11631.

Instead of letting the consumer to always try to discover coordinator in pool with either mode (subscribe / assign), we defer the clearance of discover future upon committing async only. More specifically, under manual assign mode, there are only three places where we need the coordinator:

commitAsync (both by the consumer itself or triggered by caller), this is where we want to fix.
commitSync, which we already try to re-discovery coordinator.
committed (both by the consumer itself based on reset policy, or triggered by caller), which we already try to re-discovery coordinator.
The benefits are that for manual assign mode that does not try to trigger any of the above three, then we never would be discovering coordinator. The original fix in apache#11631 would let the consumer to discover coordinator even if none of the above operations are required.

Reviewers: Luke Chen <showuon@gmail.com>, David Jacot <djacot@confluent.io>

Co-authored-by: Guozhang Wang <wangguoz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants