-
Notifications
You must be signed in to change notification settings - Fork 1.8k
chore: finish up the move to atomic types #3399
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
base: main
Are you sure you want to change the base?
Conversation
| msg.Offset = pc.suppressedHighWaterMarkOffset | ||
| pc.suppressedHighWaterMarkOffset++ |
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.
Rechecked this, and yes, the usage is indeed always made under lock.
| t *testing.T | ||
| clientID string | ||
| isCapped bool | ||
| sink *testFuncConsumerGroupSink |
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.
These are set at initialisation and never changed after, and thus can be accessed freely without mutex.
| claims map[string]int | ||
| errs []error |
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.
These are the two fields that can update after initialization, and are already correctly protected by mutexes. Their movement to here is just to match convention of putting mutex protected values below the mutex that protects them.
Signed-off-by: Cassondra Foesch <[email protected]>
… at the top Signed-off-by: Cassondra Foesch <[email protected]>
1269371 to
0c3df26
Compare
This completes the change in #3277 and rolls out all atomics usage to using the types, rather than the bare functions.
This already found a few usages that had inconsistently used atomic functions on some values.