Skip to content

[PlacementGroup]Add PlacementGroup wait java api#12499

Merged
raulchen merged 19 commits intoray-project:masterfrom
antgroup:add_wait_pg_api
Dec 5, 2020
Merged

[PlacementGroup]Add PlacementGroup wait java api#12499
raulchen merged 19 commits intoray-project:masterfrom
antgroup:add_wait_pg_api

Conversation

@ffbin
Copy link
Copy Markdown
Contributor

@ffbin ffbin commented Nov 30, 2020 •
edited
Loading

Why are these changes needed?

Add placement group wait java api:

 boolean wait(int timeoutMs)

Simplify the process of judging whether PG is created successfully, and the logic is implemented in c++ layer, which can unify the API of Java and python.

Related issue number

#12617

Checks

  • 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 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 :(

@ffbin ffbin changed the title [WIP][PlacementGroup]Add WaitPlacementGroupReady java api [PlacementGroup]Add WaitPlacementGroupReady java api Nov 30, 2020
@ffbin ffbin requested a review from clay4megtr November 30, 2020 16:06
@ffbin ffbin added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Nov 30, 2020
clay4megtr
clay4megtr reviewed Dec 1, 2020
View reviewed changes
java/runtime/src/main/java/io/ray/runtime/gcs/GlobalStateAccessor.java Outdated
Copy link
Copy Markdown
Contributor

@clay4megtr clay4megtr Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 👍

clay4megtr
clay4megtr reviewed Dec 1, 2020
View reviewed changes
@ffbin ffbin changed the title [PlacementGroup]Add WaitPlacementGroupReady java api [PlacementGroup]Add PlacementGroup wait java api Dec 2, 2020
@ffbin ffbin mentioned this pull request Dec 2, 2020
Closed
@ffbin ffbin removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Dec 2, 2020
rkooo567
rkooo567 previously requested changes Dec 4, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rkooo567 rkooo567 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but there's newline issue. Please fix it and lmk!

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Dec 4, 2020

Also @ffbin Can you create an issue with milestone to implement the python API?

@rkooo567 rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Dec 4, 2020
@ffbin
Copy link
Copy Markdown
Contributor Author

ffbin commented Dec 4, 2020

Also @ffbin Can you create an issue with milestone to implement the python API?

I have create an issue, thanks.

@ffbin
Copy link
Copy Markdown
Contributor Author

ffbin commented Dec 4, 2020

LGTM, but there's newline issue. Please fix it and lmk!
hi @rkooo567 , i have fix it and pls help take a look, thanks.

@ffbin ffbin requested a review from rkooo567 December 4, 2020 13:26
@ffbin ffbin removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Dec 4, 2020
@ffbin ffbin dismissed rkooo567’s stale review December 5, 2020 08:27

We will merge this PR first. If you have comments, we can implement it in the next PR(python API), thanks.

@raulchen raulchen merged commit 260b07c into ray-project:master Dec 5, 2020
@raulchen raulchen deleted the add_wait_pg_api branch December 5, 2020 08:40
@fishbone fishbone mentioned this pull request Dec 7, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raulchen raulchen raulchen approved these changes

@clay4megtr clay4megtr clay4megtr left review comments

@rkooo567 rkooo567 Awaiting requested review from rkooo567

Assignees

@clay4megtr clay4megtr

@rkooo567 rkooo567

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ffbin @rkooo567 @raulchen @clay4megtr