[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized #37117

scv119 · 2023-07-05T20:42:29Z

Why are these changes needed?

there is a race condition where grpc server start handling requests before the core worker is initialized. This PR fixes by waiting for initialization before handling any grpc request.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

src/ray/core_worker/core_worker.cc

rkooo567

I think it'd be the best if we can fix by changing the initialization order as @jjyao said. But if not, I have comments for existing logics;

src/ray/core_worker/core_worker.cc

scv119 · 2023-07-06T03:37:08Z

yes it's from GCS, let me defer that instead.

scv119 · 2023-07-06T05:27:19Z

huh none of them seems easy. we should process with this.

fishbone · 2023-07-06T18:31:06Z

I'm OK with the solution but preferred with the other way like fixing the initialization. I'll leave it to @jjyao @rkooo567

rkooo567

Looks great to me!

scv119 · 2023-07-11T08:19:28Z

src/ray/rpc/server_call.h

@@ -196,6 +209,9 @@ class ServerCallImpl : public ServerCall {
  }

  void HandleRequestImpl() {
+    if constexpr (std::is_base_of_v<DelayedServiceHandler, ServiceHandler>) {


C++ is amazing

…ore worker not yet initialized (ray-project#37117) Why are these changes needed? there is a race condition where grpc server start handling requests before the core worker is initialized. This PR fixes by waiting for initialization before handling any grpc request.

#37301) * [Core] Fix the race condition where grpc requests are handled while core worker not yet initialized (#37117) Why are these changes needed? there is a race condition where grpc server start handling requests before the core worker is initialized. This PR fixes by waiting for initialization before handling any grpc request. * update

… while core worker not yet initialized (#37117)" This reverts commit 5e8fd37.

… while core worker not yet initialized (#37117)" (#37343) This reverts commit 5e8fd37.

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" This reverts commit 59a15fc.

… handled while core worker not yet initialized (#37117)" (#37343)" (#37495) This reverts commit 59a15fc.

… while core worker not yet initialized (ray-project#37117)" (ray-project#37343) This reverts commit 5e8fd37.

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" (ray-project#37495) This reverts commit 59a15fc.

…ore worker not yet initialized (ray-project#37117) Why are these changes needed? there is a race condition where grpc server start handling requests before the core worker is initialized. This PR fixes by waiting for initialization before handling any grpc request. Signed-off-by: NripeshN <nn2012@hw.ac.uk>

… while core worker not yet initialized (ray-project#37117)" (ray-project#37343) This reverts commit 5e8fd37. Signed-off-by: NripeshN <nn2012@hw.ac.uk>

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" (ray-project#37495) This reverts commit 59a15fc. Signed-off-by: NripeshN <nn2012@hw.ac.uk>

…ore worker not yet initialized (ray-project#37117) Why are these changes needed? there is a race condition where grpc server start handling requests before the core worker is initialized. This PR fixes by waiting for initialization before handling any grpc request. Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

… while core worker not yet initialized (ray-project#37117)" (ray-project#37343) This reverts commit 5e8fd37. Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" (ray-project#37495) This reverts commit 59a15fc. Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" (ray-project#37495) This reverts commit 59a15fc. Signed-off-by: Victor <vctr.y.m@example.com>

scv119 added 2 commits July 5, 2023 13:40

update

0144e98

update

4aef985

scv119 assigned fishbone and rkooo567 Jul 5, 2023

scv119 linked an issue Jul 5, 2023 that may be closed by this pull request

[release-test] placement_group_performance_test.aws failed #36829

Closed

scv119 marked this pull request as ready for review July 5, 2023 20:43

jjyao reviewed Jul 5, 2023

View reviewed changes

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

scv119 added 2 commits July 5, 2023 15:54

api

a981dc2

update

a428987

rkooo567 reviewed Jul 6, 2023

View reviewed changes

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

scv119 added the do-not-merge Do not merge this PR! label Jul 6, 2023

update

9419e3a

scv119 removed the do-not-merge Do not merge this PR! label Jul 6, 2023

scv119 added 2 commits July 5, 2023 22:53

update

9a68398

Merge remote-tracking branch 'upstream/master' into fix-pg-1

7350347

scv119 force-pushed the fix-pg-1 branch from b09c3ff to 7350347 Compare July 6, 2023 05:55

rkooo567 approved these changes Jul 7, 2023

View reviewed changes

scv119 added the do-not-merge Do not merge this PR! label Jul 9, 2023

scv119 added 4 commits July 11, 2023 00:13

update

3f9c247

update

65b5cfe

update

4ac577d

update

f2942c2

scv119 commented Jul 11, 2023

View reviewed changes

scv119 merged commit 5e8fd37 into ray-project:master Jul 11, 2023

scv119 mentioned this pull request Jul 11, 2023

[Core] Fix the race condition where grpc requests are handled while c… #37301

Merged

8 tasks

scv119 added a commit that referenced this pull request Jul 12, 2023

Revert "[Core] Fix the race condition where grpc requests are handled…

ecf2623

… while core worker not yet initialized (#37117)" This reverts commit 5e8fd37.

scv119 mentioned this pull request Jul 12, 2023

Revert "[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized" #37343

Merged

scv119 added a commit that referenced this pull request Jul 12, 2023

Revert "[Core] Fix the race condition where grpc requests are handled…

59a15fc

… while core worker not yet initialized (#37117)" (#37343) This reverts commit 5e8fd37.

scv119 added a commit to scv119/ray that referenced this pull request Jul 12, 2023

Revert "Revert "[Core] Fix the race condition where grpc requests are…

8ea9d27

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" This reverts commit 59a15fc.

rkooo567 added a commit to rkooo567/ray that referenced this pull request Jul 18, 2023

Revert "Revert "[Core] Fix the race condition where grpc requests are…

c34787d

… handled while core worker not yet initialized (ray-project#37117)" (ray-project#37343)" This reverts commit 59a15fc.

rkooo567 mentioned this pull request Jul 18, 2023

Revert "Revert "[Core] Fix the race condition where grpc requests are… #37495

Merged

8 tasks

scv119 pushed a commit that referenced this pull request Jul 18, 2023

Revert "Revert "[Core] Fix the race condition where grpc requests are…

6a8d0f4

… handled while core worker not yet initialized (#37117)" (#37343)" (#37495) This reverts commit 59a15fc.

Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023

Revert "[Core] Fix the race condition where grpc requests are handled…

28e3441

… while core worker not yet initialized (ray-project#37117)" (ray-project#37343) This reverts commit 5e8fd37.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized #37117

[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized #37117

scv119 commented Jul 5, 2023 •

edited

Loading

rkooo567 left a comment

scv119 commented Jul 6, 2023

scv119 commented Jul 6, 2023

fishbone commented Jul 6, 2023

rkooo567 left a comment

scv119 Jul 11, 2023

[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized #37117

[Core] Fix the race condition where grpc requests are handled while core worker not yet initialized #37117

Conversation

scv119 commented Jul 5, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

rkooo567 left a comment

Choose a reason for hiding this comment

scv119 commented Jul 6, 2023

scv119 commented Jul 6, 2023

fishbone commented Jul 6, 2023

rkooo567 left a comment

Choose a reason for hiding this comment

scv119 Jul 11, 2023

Choose a reason for hiding this comment

scv119 commented Jul 5, 2023 •

edited

Loading