incubator-uniffle icon indicating copy to clipboard operation
incubator-uniffle copied to clipboard

[#2261]: feat(Coordinator): Introduce banned id manager and checker

Open maobaolong opened this issue 1 year ago • 7 comments

What changes were proposed in this pull request?

Introduce banned id manager and checker

Why are the changes needed?

Provide a common way to banned an app to fallback to SORT shuffle manager.

The app id can be extracted from a specific spark conf.

Fix: #2261

Does this PR introduce any user-facing change?

$  curl -X POST http://localhost:19998/banned/reload \
-H "Content-Type: application/json" \
-d '{"version": "v1.0", "ids": ["123", "456", "789"]}'

{"code":0,"data":"success","errMsg":"success"}%                                                                                                                                                                                                                                                                                                                           

$  curl http://localhost:19998/banned/version

{"code":0,"data":"v1.0","errMsg":"success"}%

How was this patch tested?

New added UT.

maobaolong avatar Nov 18 '24 12:11 maobaolong

Test Results

 2 971 files  +15   2 971 suites  +15   6h 28m 13s ⏱️ ±0s  1 094 tests + 2   1 092 ✅ + 2   2 💤 ±0  0 ❌ ±0  13 715 runs  +30  13 685 ✅ +30  30 💤 ±0  0 ❌ ±0 

Results for commit ea1f3b0d. ± Comparison against base commit 99eaa9d2.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Nov 18 '24 12:11 github-actions[bot]

Thanks for proposing this, I think I will review this in next days.

zuston avatar Nov 19 '24 02:11 zuston

You would better create an issue for a feature or a bug.

jerqi avatar Nov 25 '24 02:11 jerqi

@jerqi Thank you for remind me this, I created an issue mark it as a feature.

maobaolong avatar Nov 25 '24 04:11 maobaolong

@zuston Would you like to take a look this week? Thank you.

maobaolong avatar Dec 02 '24 14:12 maobaolong

You would better create an issue for a feature or a bug.

Sorry for the late reply, I'm struggle with the busy work. I will review this tonight.

zuston avatar Dec 04 '24 10:12 zuston

I do a quick review about this issue, and I hope I have gotten your motivation that you want to remote control the spark app to determinze whether to use uniffle or sort based shuffle manager. If so, I think the current code base have provided these ability to implement custom plugin to control this.

For the client side, you could implement the custom shuffle manager to extend the DelegationShuffleManager to inject the extra info into the access request's extraProperties protobuf var.

For the coordinator side, you also could implement custom checker to retrieve above extra properties to control app pass or not.

zuston avatar Dec 04 '24 12:12 zuston