[core] Guard concurrent access to generator IDs with a mutex #50740
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
edoakes
force-pushed
the
eoakes/test
branch
2 times, most recently
from
714f2fd to
b875ecc
Compare
This reverts commit 7fae1f9. Signed-off-by: Edward Oakes <[email protected]>
…n Python thread (ray-project#49547)" This reverts commit 41d15db. Signed-off-by: Edward Oakes <[email protected]>
… non-main Python thread (ray-project#49547)"" This reverts commit 97a0127. Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
edoakes
changed the title
edoakes
commented
Feb 22, 2025
release/release_tests.yaml
Outdated
Comment on lines
2059
to
2068
| - __suffix__: aws0 | ||
| - __suffix__: aws1 | ||
| - __suffix__: aws2 | ||
| - __suffix__: aws3 | ||
| - __suffix__: aws4 | ||
| - __suffix__: aws5 | ||
| - __suffix__: aws6 | ||
| - __suffix__: aws7 | ||
| - __suffix__: aws8 | ||
| - __suffix__: aws9 |
There was a problem hiding this comment.
I will remove this before merging, it's there for verifying that it fixes the issue :)
|
@stephanie-wang PTAL Also -- is there a difference between an "object ref stream" and an "object ref generator"? They seem to be used mostly interchangeably in the code which was a bit confusing. For example: |
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
dentiny
reviewed
Feb 23, 2025
dentiny
reviewed
Feb 23, 2025
dentiny
approved these changes
Feb 23, 2025
Signed-off-by: Edward Oakes <[email protected]>
edoakes
commented
Feb 23, 2025
src/ray/core_worker/core_worker.h
Outdated
Comment on lines
125
to
135
| ABSL_GUARDED_BY(mu_); | ||
|
|
||
| // Additionally tracks the sub-states of RUNNING_IN_RAY_GET/WAIT. The counters here | ||
| // overlap with those of counter_. | ||
| CounterMap<std::pair<std::string, bool>> running_in_get_counter_ ABSL_GUARDED_BY(&mu_); | ||
| CounterMap<std::pair<std::string, bool>> running_in_wait_counter_ ABSL_GUARDED_BY(&mu_); | ||
| CounterMap<std::pair<std::string, bool>> running_in_get_counter_ ABSL_GUARDED_BY(mu_); | ||
| CounterMap<std::pair<std::string, bool>> running_in_wait_counter_ ABSL_GUARDED_BY(mu_); | ||
|
|
||
| std::string job_id_ ABSL_GUARDED_BY(&mu_); | ||
| std::string job_id_ ABSL_GUARDED_BY(mu_); | ||
| // Used for actor state tracking. | ||
| std::string actor_name_ ABSL_GUARDED_BY(&mu_); | ||
| int64_t num_tasks_running_ ABSL_GUARDED_BY(&mu_) = 0; | ||
| std::string actor_name_ ABSL_GUARDED_BY(mu_); | ||
| int64_t num_tasks_running_ ABSL_GUARDED_BY(mu_) = 0; |
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
|
Release tests passed 20/20: https://buildkite.com/ray-project/release/builds/33907 |
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
edoakes
added a commit
to edoakes/ray
that referenced
this pull request
Feb 23, 2025
…ject#50740) The serve microbenchmark has been sporadically failing due to memory corruption issues (see the linked issue). One of the tracebacks captured pointed to the fact that the `deleted_generator_ids_` map was being accessed concurrently by multiple threads. Fixed by adding a mutex. Verified that it at least dramatically reduces the frequency of the crashes. I've also renamed a few fields for clarity. ## Related issue number ray-project#50802 --------- Signed-off-by: Edward Oakes <[email protected]>
8 tasks
edoakes
added a commit
to edoakes/ray
that referenced
this pull request
Feb 23, 2025
…ject#50740) The serve microbenchmark has been sporadically failing due to memory corruption issues (see the linked issue). One of the tracebacks captured pointed to the fact that the `deleted_generator_ids_` map was being accessed concurrently by multiple threads. Fixed by adding a mutex. Verified that it at least dramatically reduces the frequency of the crashes. I've also renamed a few fields for clarity. ray-project#50802 --------- Signed-off-by: Edward Oakes <[email protected]>
aslonnie
pushed a commit
that referenced
this pull request
Feb 23, 2025
Cherry-pick #50740, fixes some serve test flakiness Signed-off-by: Edward Oakes <[email protected]>
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.
Why are these changes needed?
The serve microbenchmark has been sporadically failing due to memory corruption issues (see the linked issue). One of the tracebacks captured pointed to the fact that the
deleted_generator_ids_map was being accessed concurrently by multiple threads. Fixed by adding a mutex.Verified that it at least dramatically reduces the frequency of the crashes.
I've also renamed a few fields for clarity.
Related issue number
#50802