Skip to content

RATIS-2685. Follower gauges are stale after LogAppender restart - #1587

Merged
szetszwo merged 4 commits into
apache:masterfrom
symious:RATIS-2685
Sep 15, 2026
Merged

szetszwo merged 4 commits into
apache:masterfrom
symious:RATIS-2685

Conversation

@symious

@symious symious commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

When a LogAppender is restarted, a new FollowerInfo is created, but the old follower gauges (follower_<id>_next_index, match_index, rpc_response_time) are never removed. The registry does not replace an existing gauge, so these gauges keep reading the stopped FollowerInfo and stop changing.

This PR removes the follower gauges when senders are stopped and before re-registering them.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2685

How was this patch tested?

Added TestLogAppenderMetrics#testFollowerGaugesAfterReAdd, which fails without the fix.

🤖 Generated with Claude Code

symious and others added 2 commits September 14, 2026 15:13
LeaderStateImpl.restart() creates a new FollowerInfo for the same follower,
but the follower gauges registered for the old FollowerInfo are never
removed, and the registry does not replace an existing gauge. The gauges
keep reading the stopped FollowerInfo.

Remove the follower gauges when senders are stopped and before
re-registering them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@symious , thanks for working on this! Please see the comments inlined.

Comment thread ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java Outdated
Replace the follower gauges in LogAppenderMetrics and do not remove them
in LeaderStateImpl.stopAndRemoveSenders.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@symious

symious commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@szetszwo Test passed locally, could you help to retry the failed test?

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 2b52eba into apache:master Sep 15, 2026
31 of 32 checks passed
@symious

symious commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@szetszwo Thank you for the review and merge.

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.

2 participants