-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 condition in reading the dropped spans number #2615
Conversation
As any race condition this should be consider an undefined behavior, and a patch release should be done. Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
ac6561a
to
fb82c93
Compare
Codecov Report
@@ Coverage Diff @@
## main #2615 +/- ##
=====================================
Coverage 76.2% 76.2%
=====================================
Files 173 173
Lines 12238 12238
=====================================
Hits 9328 9328
Misses 2667 2667
Partials 243 243
|
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
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.
While the synchornization logic behind bsp.dropped
could be improved I still see value in this PR.
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.
atomic.LoadUint32(&bsp.dropped)
could be a marker that indicates a logging point for dropped spans while refactoring the synchronization logic.
Since this is an important bug fix, can you please merge and release 1.4.1. This blocks downstream dependencies to upgrade, since tools will block the upgrade because of race condition detected. |
Working on the release now. |
As any race condition this should be consider an undefined behavior, and a patch release should be done.
Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com