[batch] job manager handles job state transition #180
Conversation
|
Can you create some issues to track the progress? each PR should be linked to one specific issues or umbrella issue. Otherwise, it's hard for me to know the scope the PR. |
I create this issue here: #182 and link it in the PR's description. |
| 1. _pending_jobs are jobs that are not scheduled yet | ||
| 2. _in_progress_jobs are jobs that are in progress now. | ||
| Theses are the input to the job scheduler. | ||
| 3. _done_jobs are inactive jobs. This needs to be updated periodically. |
There was a problem hiding this comment.
any reason done job should be updated periodically?
There was a problem hiding this comment.
The reason is to remove outdated jobs. For example, it only stores the jobs for the past 1 day. Otherwise, this results in memory leak as more and more jobs are accumulated.
There was a problem hiding this comment.
this sounds tricky, In that case, done job would be only updated once, right? not periodically?
| del self._pending_jobs[job_id] | ||
| return True | ||
|
|
||
| def mark_job_progress(self, job_id, executed_requests): |
There was a problem hiding this comment.
seems mark_job_progress and following methods are all in-memory operations, do you plan to persist the status?
There was a problem hiding this comment.
Yes, the jobs and progress are planned to be persist in storage.
There was a problem hiding this comment.
Let's have some follow up task to track this part.
| Theses are the input to the job scheduler. | ||
| 3. _done_jobs are inactive jobs. This needs to be updated periodically. | ||
| """ | ||
| self._pending_jobs = {} |
There was a problem hiding this comment.
If the job_manager is down, how do you recover the status?
There was a problem hiding this comment.
Now I have one function: "def sync_job_to_storage(self, jobId)". There should be another constructor from storage. This is a TODO. I'll try to have a first E2E ASAP.
|
Comments have been addressed now. @Jeffwan |
|
beside the design questions we discussed, rest part looks good to me. Feel free to merge it. Let's track those stories and implement them |
Ack |
* job manager handles job metadata and state transition * update comments * format check is done * update comment --------- Co-authored-by: xin.chen <[email protected]>
Pull Request Description
[Please provide a clear and concise description of your changes here]
Related Issues
Resolves: part of issue #182
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.