You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "symious (via GitHub)" <gi...@apache.org> on 2026/09/14 07:16:26 UTC
[PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
symious opened a new pull request, #1587:
URL: https://github.com/apache/ratis/pull/1587
## 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](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #1587:
URL: https://github.com/apache/ratis/pull/1587
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1587:
URL: https://github.com/apache/ratis/pull/1587#discussion_r4008185866
##########
ratis-server/src/main/java/org/apache/ratis/server/metrics/LogAppenderMetrics.java:
##########
@@ -47,8 +47,16 @@ private static RatisMetricRegistry createRegistry(String serverId) {
public void addFollowerGauges(RaftPeerId id, LongSupplier getNextIndex, LongSupplier getMatchIndex,
Supplier<Timestamp> getLastRpcTime) {
+ // The registry does not replace an existing gauge; remove it so that the new suppliers take effect.
+ removeFollowerGauges(id);
getRegistry().gauge(String.format(FOLLOWER_NEXT_INDEX, id), () -> getNextIndex::getAsLong);
getRegistry().gauge(String.format(FOLLOWER_MATCH_INDEX, id), () -> getMatchIndex::getAsLong);
getRegistry().gauge(String.format(FOLLOWER_RPC_RESP_TIME, id), () -> () -> getLastRpcTime.get().elapsedTimeMs());
}
+
+ public void removeFollowerGauges(RaftPeerId id) {
+ getRegistry().remove(String.format(FOLLOWER_NEXT_INDEX, id));
+ getRegistry().remove(String.format(FOLLOWER_MATCH_INDEX, id));
+ getRegistry().remove(String.format(FOLLOWER_RPC_RESP_TIME, id));
+ }
Review Comment:
Let's add a private replaceGauge method:
```java
public void addFollowerGauges(RaftPeerId id, LongSupplier getNextIndex, LongSupplier getMatchIndex,
Supplier<Timestamp> getLastRpcTime) {
replaceGauge(String.format(FOLLOWER_NEXT_INDEX, id), getNextIndex);
replaceGauge(String.format(FOLLOWER_MATCH_INDEX, id), getMatchIndex);
replaceGauge(String.format(FOLLOWER_RPC_RESP_TIME, id), () -> getLastRpcTime.get().elapsedTimeMs());
}
private void replaceGauge(String name, LongSupplier supplier) {
getRegistry().remove(name);
getRegistry().gauge(name, () -> supplier::getAsLong);
}
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -698,6 +698,7 @@ private void stopAndRemoveSenders(Predicate<LogAppender> predicate) {
private void stopAndRemoveSenders(Collection<LogAppender> toStop) {
toStop.forEach(LogAppender::stopAsync);
senders.removeAll(toStop);
+ toStop.forEach(s -> logAppenderMetrics.removeFollowerGauges(s.getFollowerId()));
Review Comment:
After the change in LogAppenderMetrics, this is not needed. Let's don't add this line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #1587:
URL: https://github.com/apache/ratis/pull/1587#discussion_r4011094466
##########
ratis-server/src/main/java/org/apache/ratis/server/metrics/LogAppenderMetrics.java:
##########
@@ -47,8 +47,16 @@ private static RatisMetricRegistry createRegistry(String serverId) {
public void addFollowerGauges(RaftPeerId id, LongSupplier getNextIndex, LongSupplier getMatchIndex,
Supplier<Timestamp> getLastRpcTime) {
+ // The registry does not replace an existing gauge; remove it so that the new suppliers take effect.
+ removeFollowerGauges(id);
getRegistry().gauge(String.format(FOLLOWER_NEXT_INDEX, id), () -> getNextIndex::getAsLong);
getRegistry().gauge(String.format(FOLLOWER_MATCH_INDEX, id), () -> getMatchIndex::getAsLong);
getRegistry().gauge(String.format(FOLLOWER_RPC_RESP_TIME, id), () -> () -> getLastRpcTime.get().elapsedTimeMs());
}
+
+ public void removeFollowerGauges(RaftPeerId id) {
+ getRegistry().remove(String.format(FOLLOWER_NEXT_INDEX, id));
+ getRegistry().remove(String.format(FOLLOWER_MATCH_INDEX, id));
+ getRegistry().remove(String.format(FOLLOWER_RPC_RESP_TIME, id));
+ }
Review Comment:
Updated, PTAL.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -698,6 +698,7 @@ private void stopAndRemoveSenders(Predicate<LogAppender> predicate) {
private void stopAndRemoveSenders(Collection<LogAppender> toStop) {
toStop.forEach(LogAppender::stopAsync);
senders.removeAll(toStop);
+ toStop.forEach(s -> logAppenderMetrics.removeFollowerGauges(s.getFollowerId()));
Review Comment:
Updated, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #1587:
URL: https://github.com/apache/ratis/pull/1587#issuecomment-5675567024
@szetszwo Thank you for the review and merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2685. Follower gauges are stale after LogAppender restart [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #1587:
URL: https://github.com/apache/ratis/pull/1587#issuecomment-5673599801
@szetszwo Test passed locally, could you help to retry the failed test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org