Skip to content

Fix stop race in MonitorScheduler.#19161

Merged
gianm merged 1 commit intoapache:masterfrom
gianm:fix-monitor-stop-race
Mar 16, 2026
Merged

Fix stop race in MonitorScheduler.#19161
gianm merged 1 commit intoapache:masterfrom
gianm:fix-monitor-stop-race

Conversation

@gianm
Copy link
Contributor

@gianm gianm commented Mar 15, 2026

When MonitorScheduler "stop" is called, it in turn calls "removeMonitor". This calls "monitor" on all monitors. The whole process is synchronized on the MonitorScheduler itself, but it is not synchronized with the regularly-scheduled calls to "monitor". It can lead to "monitor" running concurrently with itself, which can lead to unpredictable behavior in metrics emission.

One likely effect of this would be to emit metrics twice, due to the common pattern of snapshot -> emit deltas from previous snapshot -> save snapshot.

This patch also removes the unused CompoundMonitor.

When MonitorScheduler "stop" is called, it in turn calls "removeMonitor".
This calls "monitor" on all monitors. The whole process is synchronized
on the MonitorScheduler itself, but it is not synchronized with the
regularly-scheduled calls to "monitor". It can lead to "monitor" running
concurrently with itself, which can lead to unpredictable behavior in
metrics emission.

One likely effect of this would be to emit metrics twice, due to the
common pattern of snapshot -> emit deltas from previous snapshot ->
save snapshot.

This patch also removes the unused CompoundMonitor.
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Nice catch!

@gianm gianm merged commit dc1c4d9 into apache:master Mar 16, 2026
62 of 63 checks passed
@gianm gianm deleted the fix-monitor-stop-race branch March 16, 2026 17:55
@github-actions github-actions bot added this to the 37.0.0 milestone Mar 16, 2026
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.

2 participants