feat: add resource utilisation middleware to monitor CPU and memory. #1352
Conversation
WalkthroughA resource monitoring and throttling feature was introduced to the HTTP server. This includes a background task that monitors CPU and memory usage, configurable thresholds and intervals via CLI, middleware to reject requests when resources are over-utilized, and integration of this middleware into key HTTP endpoints. Supporting validation and workflow cache updates were also made. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActixServer
participant ResourceMonitor
participant Middleware
Note over ResourceMonitor: Runs in background<br>periodically checks CPU/memory
ResourceMonitor->>ResourceMonitor: Update RESOURCE_CHECK_ENABLED flag
User->>ActixServer: Incoming HTTP request
ActixServer->>Middleware: Pass request to middleware
Middleware->>ResourceMonitor: Read RESOURCE_CHECK_ENABLED
alt Resources OK
Middleware->>ActixServer: Forward request to handler
else Resources over-utilized
Middleware->>User: Respond 503 Service Unavailable
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/handlers/http/resource_check.rs (2)
29-30: Hard-coded thresholds → move to configuration
CPU_UTILIZATION_THRESHOLDandMEMORY_UTILIZATION_THRESHOLDare compile-time constants.
Making them environment- or config-driven (e.g.PARSEABLE.options) allows tuning without redeploying.
57-58: IncludeRetry-Afterheader for back-pressure friendlinessWhen returning
503 Service Unavailable, consider addingRetry-Afterto hint clients when to attempt again:-return Err(ErrorServiceUnavailable(error_msg)); +let mut err = ErrorServiceUnavailable(error_msg); +err.response_mut().headers_mut().insert( + http::header::RETRY_AFTER, + http::HeaderValue::from_static("30"), // seconds +); +return Err(err);Small change, big UX improvement.
src/handlers/http/modal/mod.rs (1)
48-48: Import grouping
resource_checkis added to the giganticsuper::{…}list. No functional issue, but grouping related imports (middleware modules together) would aid readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/handlers/http/mod.rs(1 hunks)src/handlers/http/modal/mod.rs(2 hunks)src/handlers/http/resource_check.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/handlers/http/modal/mod.rs (1)
src/handlers/http/resource_check.rs (1)
check_resource_utilization_middleware(34-76)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
🔇 Additional comments (3)
src/handlers/http/resource_check.rs (1)
61-63:refresh_cpu_usageneeds a delay between calls
sysinfocomputes CPU load as a delta between two successive calls. When you performrefresh_cpu_usage()and immediately readglobal_cpu_usage()without any pause, the value is often ~0 %. If the staticSystemsuggestion above is not adopted, you’ll need at minimum:sys.refresh_cpu(); tokio::time::sleep(Duration::from_millis(200)).await; sys.refresh_cpu(); let cpu_usage = sys.global_cpu_usage();Failing to do so will under-report utilisation and the middleware might never trigger.
src/handlers/http/mod.rs (1)
39-40: Module export looks goodPublicly exposing
resource_checkintegrates the new middleware cleanly; no further action required.src/handlers/http/modal/mod.rs (1)
115-118: To verify the relative cost and early‐exit behavior of each middleware, let’s locate their implementations:#!/bin/bash # Find the shutdown‐check middleware implementation rg -n "fn check_shutdown_middleware" -n . # Find the resource‐utilization middleware implementation rg -n "fn check_resource_utilization_middleware" -n .
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/handlers/http/modal/ingest_server.rs (1)
129-132: Same duplication issue as inserver.rsSee earlier comment – ensure the resource monitor is started exactly once per process.
🧹 Nitpick comments (5)
src/option.rs (1)
178-188: Unify percentage-validation helpers and numeric types
validate_percentageduplicates most ofvalidate_disk_usagebut returnsf32while the earlier helper usesf64.
Consider consolidating both into a single generic routine and sticking to one float type (preferf64throughout for higher precision and consistency).src/handlers/http/modal/server.rs (2)
142-145: Guard against spawning multiple resource-monitor tasks
spawn_resource_monitor()is invoked here and again iningest_server.rs.
If both server roles coexist in the same binary this will start two background loops polling the sameSYS_INFO, doubling load and causing racy log noise.
Add aOnceCell/Lazyflag insideresource_checkso the monitor is spawned only once.
158-159: Silently dropping the monitor task handleThe oneshot signal is sent, but the spawned task’s
JoinHandleis not awaited. On orderly shutdown this may leak warnings if the task panics.
Store the handle andawaitit after sending the shutdown signal, similar tostartup_sync_handle.src/cli.rs (1)
320-338: Align threshold types with existing optionsCPU and memory thresholds use
f32whereas--max-disk-usage-percentusesf64.
For a consistent public API (and to avoid accidental precision loss) consider switching these tof64as well.src/handlers/http/resource_check.rs (1)
33-34: UseAtomicBoolfor cheap lock-free reads
RESOURCE_CHECK_ENABLEDis a single boolean read on every request. AnAtomicBoolwithOrdering::Relaxedwould avoid the async RwLock overhead and allocation per request.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli.rs(1 hunks)src/handlers/http/modal/ingest_server.rs(2 hunks)src/handlers/http/modal/server.rs(3 hunks)src/handlers/http/resource_check.rs(1 hunks)src/option.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: coverage
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/handlers/http/resource_check.rs (5)
41-45: Thresholds captured once – runtime config changes are ignored
cpu_thresholdandmemory_thresholdare read before the loop and never refreshed.
If these values are expected to be hot-reloaded (e.g. via SIGHUP or admin API), the monitor will keep using the stale numbers. Consider moving the reads inside the tick branch:- let cpu_threshold = PARSEABLE.options.cpu_utilization_threshold; - let memory_threshold = PARSEABLE.options.memory_utilization_threshold; + // fetched on every iteration to pick up dynamic config changes
51-58:spawn_blockingeach tick is avoidable overheadSampling three numeric fields from
sysinfois cheap and already guarded by aMutex.
Spawning a blocking task every 30 s creates overhead and needless thread context switches.- let (used_memory, total_memory, cpu_usage) = tokio::task::spawn_blocking(|| { - let sys = SYS_INFO.lock().unwrap(); - ... - }).await.unwrap(); + let (used_memory, total_memory, cpu_usage) = { + let sys = SYS_INFO.lock().unwrap(); + (sys.used_memory() as f32, + sys.total_memory() as f32, + sys.global_cpu_usage()) + };If you still worry about blocking, wrap the initial
/procscan in a background task and keep subsequent reads cheap.
70-73: Promote heavy log line todebug!(or throttle)Emitting a full resource report at
infoon every 30-second tick can flood production logs.
Either:
- change to
debug!, or- log only on state changes (already handled below), or
- throttle with
tracing::info!(rate = ...).Minor, but keeps logs actionable.
70-73: Preferf64for GB conversion to avoid precision loss
used_memory/total_memoryare cast tof32, then divided by1024³. Large byte counts can exceedf32’s 24-bit mantissa and lose precision (especially on >128 GiB hosts).- used_memory / 1024.0 / 1024.0 / 1024.0, - total_memory / 1024.0 / 1024.0 / 1024.0); + (used_memory as f64) / 1024f64.powi(3), + (total_memory as f64) / 1024f64.powi(3));
89-90: Acquire write lock once to read & update flagTwo separate lock calls (
readthenwrite) introduce an avoidable race window.
Capture the previous value while holding the write lock:- let previous_state = *RESOURCE_CHECK_ENABLED.read().await; - *RESOURCE_CHECK_ENABLED.write().await = resource_ok; + let mut guard = RESOURCE_CHECK_ENABLED.write().await; + let previous_state = *guard; + *guard = resource_ok;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/handlers/http/resource_check.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/handlers/http/resource_check.rs (1)
Learnt from: vkhinvasara
PR: parseablehq/parseable#1352
File: src/handlers/http/resource_check.rs:70-74
Timestamp: 2025-06-18T08:43:30.100Z
Learning: The sysinfo crate (version 0.30+) returns memory values from `used_memory()` and `total_memory()` in bytes, not kilobytes. Converting bytes to GiB requires dividing by 1024³ (1,073,741,824). In older versions (pre-0.30), the methods returned kilobytes.
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: coverage
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (1)
src/handlers/http/resource_check.rs (1)
33-34: ```shell
#!/bin/bash
set -eecho "=== Cargo.toml entries for Tokio ==="
rg -n '^\stokio\s=' --glob 'Cargo.toml' || echo "No direct Tokio entry in Cargo.toml"echo
echo "=== Cargo.lock entries for Tokio ==="
rg -n '^tokio ' --glob 'Cargo.lock' || echo "No Tokio entries found in Cargo.lock"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Example usage:
|
vkhinvasara
changed the title
|
…ing interval to be taken from the cli via `--resource-check-interval` flag. Also wrapped the middleware around cpu intensive query endpoints available in `query.rs`.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/handlers/http/modal/query_server.rs (2)
57-60: Good middleware integration, but consider broader scope protection.The middleware is correctly applied to the query factory and counts webscope using Actix Web's
from_fnpattern. However, consider whether other resource-intensive endpoints in the query server should also be protected.For example, endpoints like
/logstream/{logstream}/stats(line 306-310) and cluster metrics endpoints might also benefit from resource monitoring, given that the PR objectives mention the query server "utilizes CPU and memory heavily."Consider applying the middleware at a broader scope or to additional endpoints:
.service( web::scope(&base_path()) + .wrap(from_fn(resource_check::check_resource_utilization_middleware)) .service(Server::get_correlation_webscope()) - .service( - Server::get_query_factory() - .wrap(from_fn(resource_check::check_resource_utilization_middleware)) - ) + .service(Server::get_query_factory())Also applies to: 73-76
164-165: Improve resource monitor shutdown error handling.The shutdown signal ignores potential errors with
let _ = resource_shutdown_tx.send(());. While the receiver might be dropped, it's better to log shutdown issues for debugging purposes.Consider logging shutdown issues:
// Shutdown resource monitor -let _ = resource_shutdown_tx.send(()); +if let Err(_) = resource_shutdown_tx.send(()) { + tracing::debug!("Resource monitor may have already been shut down"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli.rs(1 hunks)src/handlers/http/modal/query_server.rs(4 hunks)src/handlers/http/modal/server.rs(8 hunks)src/handlers/http/resource_check.rs(1 hunks)src/option.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/option.rs
- src/handlers/http/modal/server.rs
- src/cli.rs
- src/handlers/http/resource_check.rs
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: coverage
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
🔇 Additional comments (1)
src/handlers/http/modal/query_server.rs (1)
25-25: LGTM: Import additions are correct.The imports for
resource_checkandfrom_fnmiddleware are properly added to support the resource utilization monitoring functionality.Also applies to: 32-32
…and server handlers
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/handlers/http/modal/mod.rs (1)
149-150: Consider logging resource monitor shutdown status.The shutdown signal uses
let _ = resource_shutdown_tx.send(());which silently ignores potential errors. While this might be intentional if the receiver could already be dropped, consider logging the result for better observability during shutdown sequences.- // Shutdown resource monitor - let _ = resource_shutdown_tx.send(()); + // Shutdown resource monitor + if let Err(_) = resource_shutdown_tx.send(()) { + warn!("Resource monitor receiver already dropped during shutdown"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/handlers/http/modal/ingest_server.rs(5 hunks)src/handlers/http/modal/mod.rs(3 hunks)src/handlers/http/modal/query_server.rs(3 hunks)src/handlers/http/modal/server.rs(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/handlers/http/modal/query_server.rs
- src/handlers/http/modal/ingest_server.rs
- src/handlers/http/modal/server.rs
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (2)
src/handlers/http/modal/mod.rs (2)
48-48: LGTM - Clean module import addition.The addition of
resource_checkto the import statement properly exposes the resource monitoring functionality for use in the server lifecycle.
110-112: Resource monitor startup integration looks correct.The resource monitor is properly initialized with a oneshot channel for graceful shutdown control. Starting it before the HTTP server creation ensures the monitoring infrastructure is ready when middleware begins checking resource utilization.
Example usage:
|
…leted middleware from the DELETE request
vkhinvasara
dismissed stale reviews from nikhilsinhaparseable and coderabbitai[bot]
via
da3583a
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/analytics.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: coverage
Resource Utilization Thresholds: Sending 503 response if CPU/Memory thresholds exceed.
Description
This PR enhances our ingestion pipeline by implementing an overload protection mechanism during peak usage. Specifically, when CPU or memory utilization crosses predefined thresholds, the system will proactively reject further ingestion requests with an HTTP 503 (Service Unavailable) status. This prevents uncontrolled resource consumption and cascading failures.
File Changes
resource_check.rs: This is a middleware which checks resource utilization before every ingestion request.modal/mod.rs: To wrap the above middleware.http/mod.rs: To expose said middleware.Summary by CodeRabbit
New Features
Chores