Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add grpc service of cleaning up job shuffle data for the scheduler to make it able to be triggered by client explicitly #485

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

yahoNanJing
Copy link
Contributor

Which issue does this PR close?

Closes #458 .

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@yahoNanJing yahoNanJing mentioned this pull request Oct 31, 2022
9 tasks
@andygrove
Copy link
Member

@yahoNanJing Is it possible to make this PR independent of #472 so that it can be reviewed separately?

@yahoNanJing
Copy link
Contributor Author

Hi @andygrove, there's some code depending on #472. Better to deal with the #472 first. I'll mark this PR as draft.

@yahoNanJing yahoNanJing marked this pull request as draft November 2, 2022 02:38
…e data be cleaned up to be triggered by client explicitly
@yahoNanJing yahoNanJing marked this pull request as ready for review November 3, 2022 02:00
@yahoNanJing yahoNanJing requested a review from andygrove November 3, 2022 02:00
Comment on lines +557 to +559
let msg = format!("Get query stage event loop error due to {:?}", e);
error!("{}", msg);
Status::internal(msg)
Copy link
Member

Choose a reason for hiding this comment

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

minor nit: we could introduce a function to create these Status messages to avoid duplicating this block of code

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @yahoNanJing. We may want to expose this from the REST API as well in the future so that we can invoke this from the UI.

@andygrove andygrove merged commit 0cddc1d into apache:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add grpc service for the scheduler to make it able to be triggered by client explicitly
3 participants