Skip to content

KAFKA-12462: proceed with task revocation in case of thread in PENDING_SHUTDOWN#10311

Merged
ableegoldman merged 2 commits intoapache:trunkfrom
ableegoldman:12462-handle-rebalance-after-thread-in-PENDING_SHUTDOWN
Mar 13, 2021
Merged

KAFKA-12462: proceed with task revocation in case of thread in PENDING_SHUTDOWN#10311
ableegoldman merged 2 commits intoapache:trunkfrom
ableegoldman:12462-handle-rebalance-after-thread-in-PENDING_SHUTDOWN

Conversation

@ableegoldman
Copy link
Copy Markdown
Member

@ableegoldman ableegoldman commented Mar 13, 2021 •
edited
Loading

Should be cherrypicked back to 2.7 & 2.8

@ableegoldman
Copy link
Copy Markdown
Member Author

ableegoldman commented Mar 13, 2021

@vvcephei @wcarlson5 @lct45

wcarlson5
wcarlson5 reviewed Mar 13, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@wcarlson5 wcarlson5 left a comment

Choose a reason for hiding this comment

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

Thanks for the super quick PR. Just had a couple of comments

Comment thread ...ams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsRebalanceListener.java
if (streamThread.setState(State.PARTITIONS_REVOKED) != null && !partitions.isEmpty()) {
// We need to still invoke handleRevocation if the thread has been told to shut down, but we shouldn't ever
// transition away from PENDING_SHUTDOWN once it's been initiated (to anything other than DEAD)
if ((streamThread.setState(State.PARTITIONS_REVOKED) != null || streamThread.state() == State.PENDING_SHUTDOWN) && !partitions.isEmpty()) {
Copy link
Copy Markdown
Contributor

@wcarlson5 wcarlson5 Mar 13, 2021

Choose a reason for hiding this comment

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

do we need to be concerned about the oder these execute?

Copy link
Copy Markdown
Member Author

@ableegoldman ableegoldman Mar 13, 2021

Choose a reason for hiding this comment

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

I think this is the correct order (assuming you mean the order of streamThread.setState(State.PARTITIONS_REVOKED) != null relative to streamThread.state() == State.PENDING_SHUTDOWN?) -- if the thread is not in PENDING_SHUTDOWN when it reaches this line, the first condition should return true, which is what we want even if it does get transitioned to PENDING_SHUTDOWN immediately after the transition to PARTITIONS_REVOKED.

Comment thread streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java Outdated
@wcarlson5
Copy link
Copy Markdown
Contributor

wcarlson5 commented Mar 13, 2021

LGTM

@ableegoldman
Copy link
Copy Markdown
Member Author

ableegoldman commented Mar 13, 2021

One unrelated test failure: kafka.network.ConnectionQuotasTest.testListenerConnectionRateLimitWhenActualRateAboveLimit()

Going to merge

@ableegoldman ableegoldman merged commit 4fe4cdc into apache:trunk Mar 13, 2021
ableegoldman added a commit that referenced this pull request Mar 13, 2021
@ableegoldman
KAFKA-12462: proceed with task revocation in case of thread in PENDIN…
2ec98a4 
…G_SHUTDOWN (#10311)

Always invoke TaskManager#handleRevocation when the thread is in PENDING_SHUTDOWN

Reviewers: Walker Carlson <wcarlson@confluent.io>
ableegoldman added a commit that referenced this pull request Mar 13, 2021
@ableegoldman
KAFKA-12462: proceed with task revocation in case of thread in PENDIN…
aef800a 
…G_SHUTDOWN (#10311)

Always invoke TaskManager#handleRevocation when the thread is in PENDING_SHUTDOWN

Reviewers: Walker Carlson <wcarlson@confluent.io>
@ableegoldman
Copy link
Copy Markdown
Member Author

ableegoldman commented Mar 13, 2021

Merged to trunk and cherrypicked to 2.7 & 2.8 cc @vvcephei

lct45 pushed a commit to confluentinc/kafka that referenced this pull request Mar 15, 2021
@ableegoldman
KAFKA-12462: proceed with task revocation in case of thread in PENDIN…
9aeee94 
…G_SHUTDOWN (apache#10311)

Always invoke TaskManager#handleRevocation when the thread is in PENDING_SHUTDOWN

Reviewers: Walker Carlson <wcarlson@confluent.io>
@ijuma ijuma mentioned this pull request Mar 17, 2021
Merged
3 tasks
ijuma added a commit to confluentinc/kafka that referenced this pull request Mar 17, 2021
@ijuma
CONFLUENT: Sync from apache/kafka/trunk (17 March 2021)
bea34ef 
Conflicts:
* Jenkinsfile: `install` -> `publishToMavenLocal`, drop ARM build and
other changes that don't make sense for Confluent's version of
`Jenkinsfile`.
* build.gradle: keep Confluent changes for automatic skipping signing
for specific version patterns (upstream only does it if the version ends
with `SNAPSHOT`).

Commits:
* apache-github/trunk: (59 commits)
  MINOR: Remove redundant allows in import-control.xml (apache#10339)
  MINOR: remove some specifying types in tool command (apache#10329)
  KAFKA-12455: Fix OffsetValidationTest.test_broker_rolling_bounce failure with Raft (apache#10322)
  MINOR: Add toString to various Kafka Metrics classes (apache#10330)
  KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full (apache#10318)
  KAFKA-12427: Don't update connection idle time for muted connections (apache#10267)
  MINOR; Various code cleanups (apache#10319)
  HOTFIX: timeout issue in removeStreamThread() (apache#10321)
  revert stream logging level back to ERROR (apache#10320)
  KAFKA-12352: Make sure all rejoin group and reset state has a reason (apache#10232)
  KAFKA-10348: Share client channel between forwarding and auto creation manager (apache#10135)
  MINOR: Update year in NOTICE (apache#10308)
  KAFKA-12398: Fix flaky test `ConsumerBounceTest.testClose` (apache#10243)
  MINOR: Remove redundant inheritance from FilteringJmxReporter #onMetricRemoved (apache#10303)
  KAFKA-12462: proceed with task revocation in case of thread in PENDING_SHUTDOWN (apache#10311)
  KAFKA-12460; Do not allow raft truncation below high watermark (apache#10310)
  MINOR: Log project, gradle, java and scala versions at the start of the build (apache#10307)
  KAFKA-10357: Add missing repartition topic validation (apache#10305)
  MINOR: Improve error message in MirrorConnectorsIntegrationBaseTest (apache#10268)
  MINOR: Add missing unit tests for Mirror Connect (apache#10192)
  ...
vvcephei pushed a commit to confluentinc/kafka that referenced this pull request Mar 20, 2021
@ableegoldman
KAFKA-12462: proceed with task revocation in case of thread in PENDIN…
e9fa1c8 
…G_SHUTDOWN (apache#10311)

Always invoke TaskManager#handleRevocation when the thread is in PENDING_SHUTDOWN

Reviewers: Walker Carlson <wcarlson@confluent.io>
@wcarlson5
Copy link
Copy Markdown
Contributor

wcarlson5 commented Mar 1, 2022

Can we cherry pick with back to 2.6 as well?

ableegoldman added a commit that referenced this pull request Mar 1, 2022
@ableegoldman
KAFKA-12462: proceed with task revocation in case of thread in PENDIN…
3371d7f 
…G_SHUTDOWN (#10311)

Always invoke TaskManager#handleRevocation when the thread is in PENDING_SHUTDOWN

Reviewers: Walker Carlson <wcarlson@confluent.io>
@ableegoldman
Copy link
Copy Markdown
Member Author

ableegoldman commented Mar 1, 2022

Can we cherry pick with back to 2.6 as well?

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wcarlson5 wcarlson5 wcarlson5 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ableegoldman @wcarlson5