[buck2]ADD: Implemented V2 Queue for Building Tasks #2024
[buck2]ADD: Implemented V2 Queue for Building Tasks #2024benjamin-747 merged 7 commits intoweb3infra-foundation:mainfrom
Conversation
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0e999557c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| { | ||
| let mut queue = self.pending_tasks.lock().await; | ||
| queue.enqueue_v2(pending_build_event)?; |
There was a problem hiding this comment.
The new v2 path stores requests in queue_v2 via enqueue_task_with_build_id_v2, but the dispatcher still only drains the legacy queue (process_pending_tasks calls queue.dequeue() on queue). In the no-idle-worker path, /v2/task now returns 200 with status: "queued", yet those tasks never become dispatchable because nothing consumes queue_v2.
Useful? React with 👍 / 👎.
| .route("/v2/task/{cl}", get(task_get_handler)) | ||
| .route("/v2/build-events/{task_id}", get(build_event_get_handler)) | ||
| .route("/v2/targets/{task_id}", get(targets_get_handler)) | ||
| .route("/v2/build-state/{build_id}", get(build_state_handler)) |
There was a problem hiding this comment.
This change removes the /v2/builds/{build_id}/logs route (and its handler in this file), which is a breaking API regression for callers that still fetch complete build logs through that v2 endpoint. The generated client surface in the repo still includes this route, so requests will now fail with 404 unless clients are migrated in lockstep.
Useful? React with 👍 / 👎.
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6abc3c13f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| { | ||
| let mut queue = self.pending_tasks.lock().await; | ||
| queue.enqueue_v2(pending_build_event)?; |
There was a problem hiding this comment.
In the new no-idle-worker v2 path, enqueue_task_with_build_id_v2 only stores the request in memory (queue_v2) and returns a build_id without creating a corresponding build_events row. Callers now receive status: "queued" and a build ID they cannot resolve via /v2/build-state/{build_id} or /v2/builds/{build_id}/logs until some later side effect inserts it (and the ID is lost entirely on process restart), which is a functional regression versus the immediate-dispatch v2 path that persists build metadata up front.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR extends the Orion server’s “v2” task flow by adding a new Uninitialized target state, introducing a v2 pending-task queue entry that carries multiple build targets, and changing the v2 task API to queue work when no idle workers are available.
Changes:
- Add
TargetState::Uninitializedand surface it in target summary responses. - Introduce
PendingBuildEventV2+TaskQueue.queue_v2and addenqueue_task_v2APIs in the scheduler. - Update
/v2/taskto enqueue instead of returning 503 when no workers are idle; relocatebuild_logs_handlerwithinapi.rs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
orion-server/src/scheduler.rs |
Adds v2 queue storage and scheduler enqueue APIs for v2 tasks. |
orion-server/src/model/targets.rs |
Introduces Uninitialized as a new target state and updates conversions/Display. |
orion-server/src/model/build_targets.rs |
Sets default latest_state to Uninitialized and adds helpers to fetch “initialized” build targets. |
orion-server/src/api.rs |
Queues v2 tasks when no idle workers exist; adds uninitialized to target summary DTO; moves build log handler definition. |
| pub async fn enqueue_task_v2( | ||
| &self, | ||
| task_id: Uuid, | ||
| cl_link: &str, | ||
| repo: String, | ||
| changes: Vec<Status<ProjectRelativePath>>, | ||
| retry_count: i32, | ||
| ) -> Result<Uuid, String> { | ||
| let build_event_id = Uuid::now_v7(); | ||
|
|
||
| self.enqueue_task_with_build_id_v2( | ||
| build_event_id, | ||
| task_id, | ||
| cl_link, | ||
| repo, | ||
| changes, | ||
| retry_count, | ||
| ) | ||
| .await?; | ||
|
|
||
| Ok(build_event_id) | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] | ||
| pub async fn enqueue_task_with_build_id_v2( | ||
| &self, | ||
| build_event_id: Uuid, | ||
| task_id: Uuid, | ||
| cl_link: &str, | ||
| repo: String, | ||
| changes: Vec<Status<ProjectRelativePath>>, | ||
| retry_count: i32, | ||
| ) -> Result<(), String> { | ||
| let targets = | ||
| BuildTarget::find_initialized_build_targets(build_event_id, task_id, &self.conn) | ||
| .await | ||
| .map_err(|e| format!("Failed to find initialized build targets: {e}"))?; | ||
| let event = BuildEventPayload::new( | ||
| build_event_id, | ||
| task_id, | ||
| cl_link.to_string(), | ||
| repo, | ||
| retry_count, | ||
| ); | ||
|
|
||
| let pending_build_event = PendingBuildEventV2 { | ||
| event_payload: event, | ||
| targets, | ||
| changes, | ||
| created_at: Instant::now(), | ||
| }; | ||
|
|
||
| { | ||
| let mut queue = self.pending_tasks.lock().await; | ||
| queue.enqueue_v2(pending_build_event)?; | ||
| } | ||
|
|
||
| // Notify that there's a new task to process | ||
| self.task_notifier.notify_one(); | ||
| Ok(()) |
| ) -> Result<(), String> { | ||
| let targets = | ||
| BuildTarget::find_initialized_build_targets(build_event_id, task_id, &self.conn) | ||
| .await | ||
| .map_err(|e| format!("Failed to find initialized build targets: {e}"))?; | ||
| let event = BuildEventPayload::new( | ||
| build_event_id, | ||
| task_id, | ||
| cl_link.to_string(), | ||
| repo, | ||
| retry_count, | ||
| ); | ||
|
|
||
| let pending_build_event = PendingBuildEventV2 { | ||
| event_payload: event, | ||
| targets, | ||
| changes, | ||
| created_at: Instant::now(), | ||
| }; |
| } else { | ||
| tracing::info!( | ||
| "No idle workers available, attempting to enqueue task {}", | ||
| task_id | ||
| ); | ||
| match state | ||
| .scheduler | ||
| .enqueue_task_v2(task_id, &req.cl_link, req.repo, req.changes, 0) | ||
| .await | ||
| { | ||
| Ok(build_id) => { | ||
| tracing::info!("Build {}/{} queued successfully", task_id, build_id); | ||
| result = OrionBuildResult { | ||
| build_id: build_id.to_string(), | ||
| status: "queued".to_string(), | ||
| message: "Task queued for processing when workers become available".to_string(), | ||
| }; | ||
| return ( | ||
| StatusCode::OK, | ||
| Json(OrionServerResponse { | ||
| task_id: task_id.to_string(), | ||
| results: vec![result], | ||
| }), | ||
| ) | ||
| .into_response(); | ||
| } | ||
|
|
||
| Err(e) => { | ||
| tracing::warn!("Failed to queue task: {}", e); | ||
| return ( | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| Json(serde_json::json!({ | ||
| "message": format!("Unable to queue task: {}", e) | ||
| })), | ||
| ) | ||
| .into_response(); | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Aid_C <57825561+AidCheng@users.noreply.github.com>
Signed-off-by: AidCheng <cn.aiden.cheng@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6e799d805
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub fn enqueue_v2(&mut self, task: PendingBuildEventV2) -> Result<(), String> { | ||
| // Check if queue is full | ||
| if self.queue_v2.len() >= self.config.max_queue_size { | ||
| return Err("Queue is full".to_string()); | ||
| } |
There was a problem hiding this comment.
enqueue_v2 adds tasks into queue_v2, but the queue lifecycle logic still only cleans self.queue in cleanup_expired(), so v2 items never age out. In periods with no available workers, stale /v2/task requests can accumulate past max_wait_time and continue occupying capacity until max_queue_size is hit, after which new v2 submissions are rejected as "Queue is full" even though expired items should have been dropped.
Useful? React with 👍 / 👎.
3743856
Scheduler::Enqueuequeue_v2for temporary storing task request from the new version