Skip to content

[#7840] feat(core): Add job status puller and orphan file cleaner#7936

Merged
jerryshao merged 2 commits intoapache:mainfrom
jerryshao:issue-7840
Aug 7, 2025
Merged

[#7840] feat(core): Add job status puller and orphan file cleaner#7936
jerryshao merged 2 commits intoapache:mainfrom
jerryshao:issue-7840

Conversation

@jerryshao
Copy link
Contributor

What changes were proposed in this pull request?

Add the job status pulling thread and orphan file cleaner thread for job manager.

Why are the changes needed?

Fix: #7840

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add UTs.

@jerryshao jerryshao requested review from mchades and yuqi1129 August 5, 2025 07:54
@jerryshao jerryshao self-assigned this Aug 5, 2025
}

StringBuffer result = new StringBuffer();
StringBuilder result = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any thread-race problem for StringBuffer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. This is recommended by intelij idea, so I change to use StringBuilder.

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

Minor ones, others LGTM.

|| job.status() == JobHandle.Status.FAILED)
.filter(
job ->
job.finishedAt() > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

When will a job status is canceled, succeeded or failed and the finished at is 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the finished time of the completed job is the current timestamp.

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM, @mchades
Would you like to take a look?

@jerryshao jerryshao merged commit eba4808 into apache:main Aug 7, 2025
28 checks passed
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.

[Subtask] Add a background thread to poll the status periodically

2 participants