fix(scheduler): fix data race #2085
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andaaron
previously approved these changes
Nov 28, 2023
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
5 times, most recently
from
af44c0c to
9875fcc
Compare
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
9 times, most recently
from
18b72e3 to
8ea9c13
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2085 +/- ##
==========================================
+ Coverage 92.00% 92.06% +0.06%
==========================================
Files 165 165
Lines 28687 28681 -6
==========================================
+ Hits 26394 26406 +12
+ Misses 1697 1678 -19
- Partials 596 597 +1 ☔ View full report in Codecov by Sentry. |
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
2 times, most recently
from
23063eb to
bfb9204
Compare
eusebiu-constantin-petu-dbk
requested review from
Andreea-Lupu and
rchincha
as code owners
eusebiu-constantin-petu-dbk
dismissed stale reviews from Andreea-Lupu and andaaron
via
16096f1
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
from
bfb9204 to
16096f1
Compare
adodon2go
previously approved these changes
Dec 5, 2023
andaaron
previously approved these changes
Dec 5, 2023
eusebiu-constantin-petu-dbk
dismissed stale reviews from andaaron and adodon2go
via
71e977e
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
2 times, most recently
from
71e977e to
d86e4a1
Compare
andaaron
previously approved these changes
Dec 5, 2023
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
from
d86e4a1 to
efd76c9
Compare
andaaron
previously approved these changes
Dec 5, 2023
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
3 times, most recently
from
580f353 to
8781241
Compare
andaaron
reviewed
Dec 7, 2023
the problem here is that scheduler can be closed in two ways: - canceling the context given as argument to scheduler.RunScheduler() - running scheduler.Shutdown() because of this shutdown can trigger a data race between calling scheduler.inShutdown() and actually pushing tasks into the pool workers solved that by keeping a quit channel and listening on both quit channel and ctx.Done() and closing the worker chan and scheduler afterwards. Signed-off-by: Petu Eusebiu <[email protected]>
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
2 times, most recently
from
a123456 to
f3f8e4f
Compare
before this we could stop scheduler either by closing the context provided to RunScheduler(ctx) or by running Shutdown(). simplify things by getting rid of the external context in RunScheduler(). keep an internal context in the scheduler itself and pass it down to all tasks. Signed-off-by: Petu Eusebiu <[email protected]>
eusebiu-constantin-petu-dbk
force-pushed
the
fix_scheduler_data_race
branch
from
f3f8e4f to
8b0b567
Compare
andaaron
approved these changes
Dec 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue does this PR fix:
What does this PR do / Why do we need it:
If an issue # is not available please add repro steps and logs showing the issue:
Testing done on this change:
Automation added to e2e:
Will this break upgrades or downgrades?
Does this PR introduce any user-facing change?:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.