Skip to content

Conversation

CalvinConfluent
Copy link
Contributor

@CalvinConfluent CalvinConfluent commented Oct 2, 2025

Simplify the last known elr update logic. This way can make a more
robust logic.

Reviewers: Jun Rao [email protected], Chia-Ping Tsai
[email protected]

@github-actions github-actions bot added triage PRs from the community kraft small Small PRs labels Oct 2, 2025
Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@CalvinConfluent : Thanks for the PR. The code LGTM. Waiting for the CI to complete.

@github-actions github-actions bot removed the triage PRs from the community label Oct 3, 2025
@CalvinConfluent CalvinConfluent requested a review from junrao October 3, 2025 16:25
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.

LGTM, I just have one small comment left

if (!useLastKnownLeaderInBalancedRecovery || !eligibleLeaderReplicasEnabled) return;
if (record.isr() != null && record.isr().isEmpty() && (partition.lastKnownElr.length != 1 ||
partition.lastKnownElr[0] != partition.leader)) {
if (record.leader() == NO_LEADER && (partition.lastKnownElr.length == 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we could remove the redundant (). for example: (record.leader() == NO_LEADER && partition.lastKnownElr.length == 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chia7712 Thanks. Updated.

@chia7712
Copy link
Member

chia7712 commented Oct 6, 2025

will merge it tomorrow if @junrao has no further reviews

Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@CalvinConfluent : Thanks for the updated PR. LGTM

@junrao junrao merged commit 162db13 into apache:trunk Oct 6, 2025
23 checks passed
CalvinConfluent added a commit to CalvinConfluent/kafka that referenced this pull request Oct 7, 2025
Simplify the last known elr update logic. This way can make a more
robust logic.

Reviewers: Jun Rao <[email protected]>, Chia-Ping Tsai
 <[email protected]>
chia7712 pushed a commit that referenced this pull request Oct 8, 2025
Backport #20629 to 4.1 branch

Simplify the last known elr update logic. This way can make a more
robust logic.

Reviewers: Jun Rao <[email protected]>, Chia-Ping Tsai
 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants