-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix race conditions when running many instances of the Agent #12342
Conversation
@@ -128,12 +128,12 @@ def instance_additional_status(): | |||
'disable_generic_tags': 'true', | |||
'additional_status': [ | |||
{ | |||
'name': "innodb_rows_read", | |||
'name': "Innodb_rows_read", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird looks like these should have been capitalized all along https://github.com/DataDog/integrations-core/blob/master/mysql/datadog_checks/mysql/const.py#L218 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm trying to figure out why this change is affecting the metrics; that's not clear to me yet, but these values are definitely wrong.
* fix STATUS_VARS to avoid using a global var * fix innodb vars d8a8f09
* fix STATUS_VARS to avoid using a global var * fix innodb vars
What does this PR do?
This removes access to mutable global vars.
Motivation
This is being reported as a bug by customers who are seeing
in the check logs. Basically what is happening is:
We set the metrics global var equal to STATUS_VARS, which is a mutable dict
This is updated at various points in this function
Sometimes these updates are conditional
... And is iterated over here
There seems to be some kind of race with another instance of the integration? As long as the code path followed every time is the same, no harm, no foul but that isn’t happening here. Making a copy of the dict before updating will prevent this possibility.
Additional Notes
Review checklist (to be filled by reviewers)
changelog/
andintegration/
labels attached