Skip to content

Revert "KAFKA-13891: reset generation when syncgroup failed with REBA…#12794

Merged
showuon merged 1 commit intoapache:trunkfrom
aiquestion:revert_kafka_13891
Nov 5, 2022
Merged

Revert "KAFKA-13891: reset generation when syncgroup failed with REBA…#12794
showuon merged 1 commit intoapache:trunkfrom
aiquestion:revert_kafka_13891

Conversation

@aiquestion
Copy link
Contributor

@aiquestion aiquestion commented Oct 28, 2022

…LANCE_IN_PROGRESS (#12140)"

This reverts commit c23d60d.

As discussed in KAFKA-14016.
The change in KAFKA-13891 will lead to revoke more partitions than expected. So revert it.

Committer Checklist (excluded from commit message)

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

Copy link
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 for the PR.

@dajac
Copy link
Member

dajac commented Nov 4, 2022

@showuon What's the plan with this one? I tend to agree that reverting it makes sense. Is there are reason why you did not merge it yet?

@showuon
Copy link
Member

showuon commented Nov 4, 2022

Ah, sorry, was waiting for jenkins build results, and ... forgot it.

@showuon
Copy link
Member

showuon commented Nov 4, 2022

Build results:

    Build / JDK 8 and Scala 2.12 / kafka.admin.TopicCommandIntegrationTest.testDeleteInternalTopic(String).quorum=kraft
    Build / JDK 8 and Scala 2.12 / kafka.api.TransactionsTest.testAbortTransactionTimeout(String).quorum=kraft
    Build / JDK 8 and Scala 2.12 / kafka.api.TransactionsTest.testCommitTransactionTimeout(String).quorum=zk
    Build / JDK 8 and Scala 2.12 / kafka.server.KRaftClusterTest.testCreateClusterAndPerformReassignment()
    Build / JDK 17 and Scala 2.13 / kafka.api.TransactionsTest.testAbortTransactionTimeout(String).quorum=zk
    Build / JDK 11 and Scala 2.13 / kafka.api.SaslPlaintextConsumerTest.testCoordinatorFailover()
    Build / JDK 11 and Scala 2.13 / kafka.api.SaslScramSslEndToEndAuthorizationTest.testNoConsumeWithoutDescribeAclViaSubscribe()
    Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testSendOffsetsToTransactionTimeout(String).quorum=kraft
    Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testAbortTransactionTimeout(String).quorum=kraft
    Build / JDK 11 and Scala 2.13 / kafka.server.KRaftClusterTest.testCreateClusterAndPerformReassignment()
    Build / JDK 11 and Scala 2.13 / kafka.server.KRaftClusterTest.testUnregisterBroker()
    Build / JDK 11 and Scala 2.13 / kafka.server.KRaftClusterTest.testUpdateMetadataVersion()
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.IQv2StoreIntegrationTest.verifyStore[cache=false, log=false, supplier=ROCKS_WINDOW, kind=DSL]
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldNotMakeStoreAvailableUntilAllStoresAvailable
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.SlidingWindowedKStreamIntegrationTest.shouldRestoreAfterJoinRestart[ON_WINDOW_UPDATE_cache:false]
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.SmokeTestDriverIntegrationTest.shouldWorkWithRebalance

I felt all are unrelated, but this test: kafka.api.TransactionsTest.testAbortTransactionTimeout failed in 3 jobs. I re-trigger the test now. Waiting for the result.
@aiquestion , in case I'll forget it, please remind me after build completed. :)

@dajac
Copy link
Member

dajac commented Nov 4, 2022

Last build had two failures:

Build / JDK 11 and Scala 2.13 / testLargeAssignmentAndGroupWithNonEqualSubscription() – org.apache.kafka.clients.consumer.CooperativeStickyAssignorTest
1m 16s
Build / JDK 17 and Scala 2.13 / testTransactionAfterTransactionIdExpiresButProducerIdRemains(String).quorum=zk – kafka.api.ProducerIdExpirationTest

cc @showuon

@showuon showuon merged commit fcab5fb into apache:trunk Nov 5, 2022
@dajac
Copy link
Member

dajac commented Nov 5, 2022

@showuon Do we need to backport it to previous releases as well?

@showuon
Copy link
Member

showuon commented Nov 5, 2022

Good point. We backed port the fix to 3.2 and 3.3 branch in #12140 . Let me backport to 3.2/3.3. Thanks for reminding.

showuon pushed a commit that referenced this pull request Nov 5, 2022
…LANCE_IN_PROGRESS (#12140)" (#12794)

This reverts commit c23d60d.

Reviewers: Luke Chen <showuon@gmail.com>
showuon pushed a commit that referenced this pull request Nov 5, 2022
…LANCE_IN_PROGRESS (#12140)" (#12794)

This reverts commit c23d60d.

Reviewers: Luke Chen <showuon@gmail.com>
guozhangwang pushed a commit to guozhangwang/kafka that referenced this pull request Jan 25, 2023
…LANCE_IN_PROGRESS (apache#12140)" (apache#12794)

This reverts commit c23d60d.

Reviewers: Luke Chen <showuon@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.

3 participants