Skip to content

MINOR: testDelayedTxnOffsetCommitWithBumpedEpochIsRejected #20024

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

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

dongnuo123
Copy link
Contributor

@dongnuo123 dongnuo123 commented Jun 24, 2025

This patch adds an API level integration test for the producer epoch verification when processing transactional offset commit.

@github-actions github-actions bot added triage PRs from the community core Kafka Broker tests Test fixes (including flaky tests) small Small PRs labels Jun 24, 2025
@dongnuo123 dongnuo123 changed the title DRAFT: testDelayedTxnOffsetCommitWithBumpedEpochIsRejected MINOR: testDelayedTxnOffsetCommitWithBumpedEpochIsRejected Jun 24, 2025
@dongnuo123 dongnuo123 marked this pull request as ready for review June 24, 2025 01:20
Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@dongnuo123 thanks for adding useful tests

// Join the consumer group. Note that we don't heartbeat here so we must use
// a session long enough for the duration of the test.
val (memberId: String, memberEpoch: Int) = joinConsumerGroup(groupId, useNewProtocol)
assertTrue(memberId != JoinGroupRequest.UNKNOWN_MEMBER_ID)
Copy link
Member

Choose a reason for hiding this comment

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

    assertNotEquals(JoinGroupRequest.UNKNOWN_MEMBER_ID, memberId)
    assertNotEquals(JoinGroupRequest.UNKNOWN_GENERATION_ID, memberEpoch)

assertNotEquals could show more readable messages.

createTopic(topic, 1)

for (version <- ApiKeys.TXN_OFFSET_COMMIT.oldestVersion to ApiKeys.TXN_OFFSET_COMMIT.latestVersion(isUnstableApiEnabled)) {
val useTV2 = version >= 5
Copy link
Member

Choose a reason for hiding this comment

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

how about using EndTxnRequest.LAST_STABLE_VERSION_BEFORE_TRANSACTION_V2 instead of magic number 5?

@@ -51,6 +51,16 @@ class TxnOffsetCommitRequestTest(cluster:ClusterInstance) extends GroupCoordinat
testTxnOffsetCommit(false)
}

@ClusterTest
def testDelayedTxnOffsetCommitWithBumpedEpochIsRejectedWithOldConsumerGroupProtocol(): Unit = {
testDelayedTxnOffsetCommitWithBumpedEpochIsRejected(true)
Copy link
Member

Choose a reason for hiding this comment

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

According to the naming, it should pass false instead of true, shouldn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

expectedError = Errors.NONE
)

// Start a new transaction. Wait for the previous transaction to complete.
Copy link
Member

Choose a reason for hiding this comment

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

Should we complete this new transaction? otherwise, the next version will have a ongoing transaction.

@github-actions github-actions bot removed the triage PRs from the community label Jun 27, 2025
Copy link
Member

@FrankYang0529 FrankYang0529 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 PR. Overall LGTM. Out of curiosity, do we want to have a similar test for EndTxnRequest and expected error for TV_2 is PRODUCER_FENCED.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved core Kafka Broker small Small PRs tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants