Skip to content

[CELEBORN-2394] Add REST v1 API to unregister shuffles - #3770

Closed
Kalvin2077 wants to merge 4 commits into
apache:mainfrom
Kalvin2077:feat/unregister-shuffle
Closed

[CELEBORN-2394] Add REST v1 API to unregister shuffles#3770
Kalvin2077 wants to merge 4 commits into
apache:mainfrom
Kalvin2077:feat/unregister-shuffle

Conversation

@Kalvin2077

@Kalvin2077 Kalvin2077 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Add an OpenAPI-based POST /api/v1/shuffles/unregister endpoint that unregisters multiple shuffles for an application in one request.
  • Generate Java client support for ShuffleApi.unregisterShuffles and UnregisterShufflesRequest from the updated OpenAPI specification.
  • Use the existing batch unregister RPC when handling the REST request.
  • Add --unregister-shuffles support to Celeborn Shell, reusing the existing --apps and --shuffleIds options.
  • Fix the legacy application resource test to deserialize ApplicationsHeartbeatResponse instead of ApplicationsResponse.

Why are the changes needed?

Shuffle unregistration was previously available only through Celeborn's internal RPC path. REST API, generated Java client, and Celeborn Shell users had no corresponding administrative operation.

Supporting multiple shuffle IDs in one request also avoids requiring callers to issue one request per shuffle and aligns the public interfaces with Celeborn's existing batch unregister RPC.

Does this PR resolve a correctness bug?

  • Yes

Does this PR introduce any user-facing change?

  • Yes

Users can unregister multiple shuffles through the REST v1 API, the generated OpenAPI Java client, or Celeborn Shell.

REST request example:

POST /api/v1/shuffles/unregister
Content-Type: application/json

{
  "appId": "app1",
  "shuffleIds": [1, 2, 3]
}

Celeborn Shell example:

celeborn-cli master --unregister-shuffles --apps app1 --shuffleIds 1,2,3

How was this patch tested?

  • Added two tests in ApiV1MasterResourceSuite covering batch unregistration, preservation of unrelated shuffles, idempotency, and invalid requests.

  • Extended ApiV1OpenapiClientSuite to cover batch unregistration through the generated Java client.

  • Added Celeborn Shell integration coverage for successful batch unregistration, missing options, multiple application IDs, malformed shuffle IDs, and negative shuffle IDs.

  • Verified the generated Java sources with:

    mvn -pl openapi/openapi-client -Pgenerate generate-resources -DskipTests
  • Ran the affected test suites with Maven. The Master REST suite passed 9 tests, the OpenAPI client suite passed 11 tests, and the CLI suite passed 43 tests with one pre-existing HA test canceled.

  • Verified formatting with:

    mvn spotless:check -pl cli -am -DskipTests

Signed-off-by: Kalvin2077 <wk.huang2077@outlook.com>
Signed-off-by: Kalvin2077 <wk.huang2077@outlook.com>
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.22449% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.24%. Comparing base (1e715b2) to head (322b2f3).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...leborn/rest/v1/model/UnregisterShuffleRequest.java 41.38% 17 Missing ⚠️
...org/apache/celeborn/rest/v1/master/ShuffleApi.java 90.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3770      +/-   ##
============================================
+ Coverage     58.05%   58.24%   +0.19%     
- Complexity      228      229       +1     
============================================
  Files           397      399       +2     
  Lines         27931    28048     +117     
  Branches       2725     2738      +13     
============================================
+ Hits          16212    16333     +121     
+ Misses        10530    10517      -13     
- Partials       1189     1198       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SteNicholas

SteNicholas commented Jul 29, 2026

Copy link
Copy Markdown
Member

@Kalvin2077, please support adding corresponding options in celeborn shell for unregistering shuffle.

@Kalvin2077 Kalvin2077 changed the title [CELEBORN-2394] Add REST v1 API to unregister a shuffle [CELEBORN-2394] Add REST v1 API to unregister shuffles Jul 29, 2026
@Kalvin2077

Kalvin2077 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@SteNicholas

  • Supported batch unregistration instead.
  • Added corresponding options to celeborn-cli.

@SteNicholas SteNicholas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@Kalvin2077
Kalvin2077 deleted the feat/unregister-shuffle branch August 3, 2026 06:12
SteNicholas pushed a commit that referenced this pull request Aug 3, 2026
### What changes were proposed in this pull request?

- Add an OpenAPI-based `POST /api/v1/shuffles/unregister` endpoint that unregisters multiple shuffles for an application in one request.
- Generate Java client support for `ShuffleApi.unregisterShuffles` and `UnregisterShufflesRequest` from the updated OpenAPI specification.
- Use the existing batch unregister RPC when handling the REST request.
- Add `--unregister-shuffles` support to Celeborn Shell, reusing the existing `--apps` and `--shuffleIds` options.
- Fix the legacy application resource test to deserialize `ApplicationsHeartbeatResponse` instead of `ApplicationsResponse`.

### Why are the changes needed?

Shuffle unregistration was previously available only through Celeborn's internal RPC path. REST API, generated Java client, and Celeborn Shell users had no corresponding administrative operation.

Supporting multiple shuffle IDs in one request also avoids requiring callers to issue one request per shuffle and aligns the public interfaces with Celeborn's existing batch unregister RPC.

### Does this PR resolve a correctness bug?

- [ ] Yes

### Does this PR introduce _any_ user-facing change?

- [x] Yes

Users can unregister multiple shuffles through the REST v1 API, the generated OpenAPI Java client, or Celeborn Shell.

REST request example:

```http
POST /api/v1/shuffles/unregister
Content-Type: application/json

{
  "appId": "app1",
  "shuffleIds": [1, 2, 3]
}
```

Celeborn Shell example:

```shell
celeborn-cli master --unregister-shuffles --apps app1 --shuffleIds 1,2,3
```

### How was this patch tested?

- Added two tests in `ApiV1MasterResourceSuite` covering batch unregistration, preservation of unrelated shuffles, idempotency, and invalid requests.
- Extended `ApiV1OpenapiClientSuite` to cover batch unregistration through the generated Java client.
- Added Celeborn Shell integration coverage for successful batch unregistration, missing options, multiple application IDs, malformed shuffle IDs, and negative shuffle IDs.
- Verified the generated Java sources with:

  ```shell
  mvn -pl openapi/openapi-client -Pgenerate generate-resources -DskipTests
  ```

- Ran the affected test suites with Maven. The Master REST suite passed 9 tests, the OpenAPI client suite passed 11 tests, and the CLI suite passed 43 tests with one pre-existing HA test canceled.
- Verified formatting with:

  ```shell
  mvn spotless:check -pl cli -am -DskipTests
  ```

Closes #3770 from Kalvin2077/feat/unregister-shuffle.

Authored-by: Kalvin2077 <wk.huang2077@outlook.com>
Signed-off-by: Nicholas Jiang <programgeek@163.com>
@SteNicholas

Copy link
Copy Markdown
Member

Thanks. Merged to main(v1.0.0) and branch-0.7(v0.7.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants