⚡ Bolt: Refactor blocking I/O to use spawn_blocking in HTTP platform handlers#334
⚡ Bolt: Refactor blocking I/O to use spawn_blocking in HTTP platform handlers#334EffortlessSteven wants to merge 2 commits intomainfrom
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors several async handlers in the http-platform crate by wrapping blocking I/O operations in tokio::task::spawn_blocking to prevent executor thread starvation. The changes affect the IDP snapshot, status, and various UI view handlers. Feedback indicates that the refactor in the get_status handler is incomplete, as several synchronous calls like load_question_counts and load_friction_counts remain on the async thread and should be consolidated into the blocking block.
| let (specs, tasks_spec, ac_cov) = tokio::task::spawn_blocking(move || { | ||
| let specs_result = load_all_specs(&root_clone); | ||
| let tasks_result = spec_runtime::load_tasks(&root_clone.join("specs/tasks.yaml")); | ||
| let ac_cov = idp::load_ac_coverage(&root_clone); | ||
| (specs_result, tasks_result, ac_cov) | ||
| }) |
There was a problem hiding this comment.
The refactor in get_status is incomplete. While some calls were moved to spawn_blocking, several other blocking I/O operations remain on the async executor thread: load_question_counts (line 264), load_friction_counts (line 267), load_fork_counts (line 270), and the fs::read_to_string for policy status (line 274). To prevent thread starvation as intended, these synchronous operations should be consolidated into the spawn_blocking block, similar to the implementation in the dashboard handler in src/ui.rs.
Test Results283 tests 245 ✅ 10m 56s ⏱️ Results for commit 6635d66. |
Scope
Type: refactor
Intent: Improve throughput by offloading blocking IO
Touchpoints: crates/http-platform/src/lib.rs, crates/http-platform/src/ui.rs, crates/http-platform/src/idp.rs
Evidence:
cargo test -p http-platformpasses💡 What:
Wrapped synchronous file system operations and YAML spec loading logic in
tokio::task::spawn_blockinginsidedashboard,graph_view,flows_view,coverage_view,debug_info,get_status, andget_idp_snapshothandlers in thehttp-platformcrate.🎯 Why:
Axum uses Tokio's async executor, which relies on a limited pool of worker threads. Performing blocking operations (like
std::fs::read_to_stringor parsing complex YAML specs viaload_all_specs) directly on these worker threads blocks them from picking up and handling other incoming HTTP requests. This leads to thread starvation, degraded performance, and significantly increased latency under load. Offloading these heavy operations tospawn_blockingensures they run on a dedicated thread pool for blocking tasks, keeping the async runtime responsive.📊 Impact:
🔬 Measurement:
/platform/dashboard,/platform/status) and observe improved latency and throughput metrics compared to baseline.PR created automatically by Jules for task 277186420635290348 started by @EffortlessSteven