-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 Concurrency issue in the metrics #4222
Conversation
A double locking was happening in recorder mutex which was creating unexpected behaviour. Lock needs to be done only in the function that calls opencensus stats recorder.
The following is the coverage report on the affected files.
|
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.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Yes, @vdemeester We probably need a e2e test or integration test to catch these type of bugs. |
e2e tests or unit tests yes 👼🏼 |
/lgtm |
A double locking was happening in recorder mutex which was creating
unexpected behavior. Lock needs to be done only in the function that
calls opencensus stats recorder. ReportRunningPipelineRuns and
ReportRunningTaskRuns calls RunningPipelineRuns and RunningTaskRuns
respectively which in turns lock the recorder.
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes