Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Procect forwarder watchOverrides from race condition #1468

Merged
merged 6 commits into from
Jun 3, 2022

Conversation

mapno
Copy link
Member

@mapno mapno commented Jun 2, 2022

What this PR does:

Reworks update logic in watchOverrides loop.
The race condition comes from the unpredictable behaviour of ranging over a map.

This could cause that a newly created queueManagers was added to the ranged map,
and be evaluated for update.
Since the queueManager was just created, it wouldn't have enough running workers,
which would trigger another update.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

I'm good on this change. Before merging please give it a test in an internal environment.

modules/distributor/forwarder.go Show resolved Hide resolved
@mapno mapno force-pushed the forwarder-race-condition branch from e28355b to c588e2c Compare June 2, 2022 16:14
mapno added 5 commits June 3, 2022 09:19
Reworks update logic in watchOverrides loop.
The race condition comes from the unpredictable behaviour of ranging over a map.

This could cause that a newly created queueManagers was added to the ranged map,
and be evaluated for update.
Since the queueManager was _just_ created, it wouldn't have enough running workers,
which would trigger another update.
@mapno mapno force-pushed the forwarder-race-condition branch from c0234b8 to 2ed7d22 Compare June 3, 2022 07:20
@mapno mapno enabled auto-merge (squash) June 3, 2022 07:21
@mapno mapno merged commit 8b7235f into grafana:main Jun 3, 2022
@mapno mapno deleted the forwarder-race-condition branch June 3, 2022 07:32
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