Sync/executor cache metrics cleanup - #159
Merged
Merged
Conversation
Exhausting a wasmtime pooling-allocator pool is not graceful: the allocator returns "maximum concurrent limit of N reached", which surfaces as an instantiation error rather than the adaptive run-queue shedding path. Sizing max_execution_stacks needed observed concurrency, which no existing metric provided. Track it with an RAII guard held in runtime::Data<T>, so the count spans exactly the lifetime of the store that owns the pooled slots, including the unwind path. Placing it there rather than in the ProxyWasm executor covers every executor sharing the single Engine — a proxywasm-only gauge would systematically undercount pool occupancy. fastedge_wasm_instances_peak is a monotonic high-water mark: a plain gauge is scrape-limited and would miss the sub-second spikes a stalled backend produces, which are the peaks the pool must be sized for.
…s, fix docs - Remove tokio_park_unpark_count: always exactly 2x tokio_park_count by construction (tokio increments it on both park and unpark), so the metric carried no information. - Remove tokio_global_queue_depth: strictly dominated by tokio_global_queue_depth_peak, which is fed by a 10ms sampler and floored at the current reading on scrape; the instantaneous value at scrape time was pure noise. - Remove tokio_busy_duration_us: tokio only publishes worker busy time just-before-park / on periodic maintenance, so the value freezes exactly when a runtime is saturated; superseded by the live-sampled fastedge_workload_15s. - fastedge_wasm_connections: IntGauge -> IntGaugeVec with a client label (nginx = V1/V2a/V2b/V3-yamux, core-proxy = V2/V2c). Counting moved from accept_loop into serve via a ConnectionGauge RAII guard, created after the handshake when the peer identity is known; Drop accounts for every exit path. - fastedge_wasm_commands_total: IntCounter -> IntCounterVec with a kind label (send = fire-and-forget, request_reply = round-trip). The request_reply path was previously not counted at all; it is now counted before the additional_info fast path, so the delta against fastedge_wasm_request_reply_duration_count measures locally-served zero-cost host calls. - Fix misleading HELP texts for fastedge_error_total_count and fastedge_wasm_memory_used (cumulative counter, not a usage gauge); names kept for backwards compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two fixes for periodic p99 latency spikes: 1. Selective invalidation on config poll: on a revision bump the poller used to call remove_all(), wiping every cached executor and forcing a cold start for all active apps every ~120s. Now Config::stale_apps() diffs old vs new config and only removed/changed apps are evicted. 2. get_executor is now async (RPITIT): executor caches switch to moka::future::Cache and the blocking build (load_component / instantiate_pre) runs via spawn_blocking instead of block_in_place, which migrated the calling worker's run queue on every cold start. moka coalesces concurrent misses for the same app into one build. Supporting changes: manual Clone for WasmEngine<T> (no T: Clone bound), ExecutorCache::remove is async (future cache invalidate is async), and config pub/sub handlers await the eviction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…alidation perf: selective executor cache invalidation and non-blocking cold starts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines executor/cache behavior and metrics/observability across the runtime and HTTP service layers, including better classification of OOM/timeout failures and new accounting for concurrently live Wasm instances (pooling allocator pressure).
Changes:
- Makes executor retrieval and cache invalidation async to support non-blocking cache misses and invalidation flows.
- Improves failure classification/metrics (OOM detection via limiter + typed error; better stats marking for mid-stream failures).
- Adds live-instance accounting metrics and expands Redis KV store support with a pooled
ConnectionManagerimplementation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/key_value.rs | Updates Redis store open call to new pooled-connection API. |
| src/context.rs | Adapts ExecutorFactory/ExecutorCache impls to async signatures. |
| crates/runtime/src/util/metrics.rs | Clarifies Prometheus metric help strings (semantics/back-compat notes). |
| crates/runtime/src/store.rs | Adds store-level OOM flag accessor and typed OutOfMemory error; wires live-instance guard into store data. |
| crates/runtime/src/limiter.rs | Tracks OOM-caused memory growth denials and adds unit tests for the flag. |
| crates/runtime/src/lib.rs | Exposes new instances module; adds WasmEngine<T> manual Clone; updates ExecutorCache trait signature/docs. |
| crates/runtime/src/instances.rs | Introduces fastedge_wasm_instances_live and peak-tracking infrastructure via RAII guard. |
| crates/runtime/src/app.rs | Adds Log::Kafka configuration option. |
| crates/key-value-store/src/redis_impl.rs | Implements a round-robin pool of ConnectionManagers for Redis KV operations. |
| crates/http-service/src/lib.rs | Refactors request tracing instrumentation; adds fail_reason_of; adjusts error mapping to detect typed OOM. |
| crates/http-service/src/executor/wasi_http.rs | Classifies instantiation OOM via limiter; records fail reason for post-header failures (e.g., mid-body trap/timeout). |
| crates/http-service/src/executor/mod.rs | Changes ExecutorFactory::get_executor to return a Future and documents motivation. |
| crates/http-service/src/executor/http.rs | Mirrors instantiation-time OOM classification; updates test ExecutorFactory implementation to async. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
qrdl
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.