add total stream/consumer counts to meta snapshot stats#7980
add total stream/consumer counts to meta snapshot stats#7980alexbozhenko wants to merge 1 commit intomainfrom
Conversation
|
Claude finished @alexbozhenko's task in 1m 42s —— View job PR Review
Two issues found: 1. Data race (critical) —
2. Stale counts when all streams are deleted — The |
| snap.LastDuration = time.Duration(durationNanos) | ||
|
|
||
| } | ||
| snap.TotalStreamCount = uint64(cluster.totalStreamCount) |
There was a problem hiding this comment.
Data race: totalStreamCount and totalConsumerCount are written with atomic.StoreInt64 in encodeMetaSnapshot, but read here without atomic loads. The race detector will flag this.
| snap.TotalStreamCount = uint64(cluster.totalStreamCount) | |
| snap.TotalStreamCount = uint64(atomic.LoadInt64(&cluster.totalStreamCount)) | |
| snap.TotalConsumerCount = uint64(atomic.LoadInt64(&cluster.totalConsumerCount)) |
This matches the existing pattern used for lastMetaSnapTime and lastMetaSnapDuration just above.
| if cc := js.cluster; cc != nil { | ||
| atomic.StoreInt64(&cc.lastMetaSnapTime, start.UnixNano()) | ||
| atomic.StoreInt64(&cc.lastMetaSnapDuration, int64(took)) | ||
| atomic.StoreInt64(&cc.totalStreamCount, int64(nsa)) |
There was a problem hiding this comment.
The if len(out) == 0 early return on the non-diff path (before this block) skips these stores. So if all streams are deleted, totalStreamCount and totalConsumerCount remain at their previous non-zero values rather than being updated to 0.
Consider moving the entire atomic-store block (or at least the count stores) to before the len(out) == 0 check, since nsa and nca are fully computed by that point.
Signed-off-by: Your Name your.email@example.com