feat(filter): add ext_proc state foundation and pipeline pinning#609
Conversation
b7e6d69 to
bf43441
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Adds per-filter typed state (filter_state HashMap) and pipeline pinning to HttpFilterContext, with stable filter_id assignment at build time.
Overall: Well-structured foundation — the current_filter_id lifecycle (set before hook, clear after) is correct, the Any + Send + Sync downcasting API is clean, and branch chain ID uniqueness tests are thorough. One issue: branch filter error handling doesn't respect FailureMode.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 1 |
| Medium | 0 |
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Adds typed per-filter state (filter_state HashMap) and stable filter invocation IDs (filter_id) to support state persistence across filter lifecycle phases.
Overall: Well-structured foundation. The filter state API is clean, ID uniqueness is thoroughly tested across branch nesting depths, and the pipeline pinning prevents hot-reload state loss. One behavioral inconsistency in branch error handling needs attention.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 1 |
| Medium | 0 |
shaneutt
left a comment
There was a problem hiding this comment.
Some small comments, and then a question:
Had you considered options to resolve this (for MCP) that were more bespoke?
593a760 to
bd1f548
Compare
This PR adds the request-scoped state and pipeline-lifecycle foundation needed for the upcoming generic `ext_proc` full-duplex integration. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
bd1f548 to
d876e03
Compare
Good idea! Kept this generic because the lifecycle-state need applies to any filter instance and avoids baking MCP/ext_proc details into the protocol handler. I’ll add MCP helpers or typed wrappers to the MCP workstream if that works. Need to think about it a bit more tbh. |
shaneutt
left a comment
There was a problem hiding this comment.
Some small comments, and then a question:
Had you considered options to resolve this (for MCP) that were more bespoke?Good idea! Kept this generic because the lifecycle-state need applies to any filter instance and avoids baking MCP/ext_proc details into the protocol handler. I’ll add MCP helpers or typed wrappers to the MCP workstream if that works. Need to think about it a bit more tbh.
Sounds good, this is OK for a follow-up PR.
Summary
This PR adds the request-scoped state and pipeline-lifecycle foundation needed for the upcoming generic
ext_procfull-duplex integration.It introduces typed per-filter state on
HttpFilterContext, keyed by a stable filter invocation ID assigned when the pipeline is built. That lets a filter store state during one lifecycle phase and read it later during request body, response, or response body processing without using globals or shared mutable state.It also pins the active
FilterPipelinefor the lifetime of each Pingora request context. This prevents hot reloads from mixing pipeline versions mid-request: a request that starts on pipeline A continues using pipeline A across later hooks, while new requests after reload use pipeline B.The working llm-d PoC integration demo, PR stack details, code walkthrough and architecture notes here: nerdalert/praxis-research-spikes
What Changed
1. Stable Filter Identity
Each
PipelineFilternow has a stablefilter_idassigned at pipeline build time.The ID is:
FilterPipelineThis is needed because request-scoped state has to belong to a specific filter instance, not just a filter type. A pipeline can contain the same filter type more than once, and branch chains can introduce additional filter instances outside the top-level pipeline loop.
Relying on only the top-level loop index is not enough because nested branch filters do not have a stable top-level index. Relying on filter type is also not enough because two
ext_procfilters, two header filters, or two branch filters of the same type must not share state.Assigning a stable
filter_idwhen the pipeline is built gives the runtime a small, deterministic key for per-request state. That key stays meaningful for the lifetime of the pinned pipeline.2. Typed Per-Request Filter State
HttpFilterContextnow carries:Filters can use typed accessors:
This gives filters a safe request-local memory slot. A filter can store state during one lifecycle callback and retrieve it later during request body, response, or response body processing.
For the upcoming full-duplex ext_proc integration, this is the foundation that allows the filter to open a processor exchange during request/header or request-body handling, then find that same exchange again when later body chunks arrive.
The map is keyed by current_filter_id, which the pipeline sets only while a filter hook is executing. That means filters do not need to manually pass keys around, and they cannot accidentally read another filter instance's state through the public accessor methods.
Box<dyn Any + Send + Sync> is used because different filters may store different concrete state types. Any allows typed downcasting through the accessor methods, while Send + Sync keeps the state compatible with PingoraRequestCtx, which must satisfy Pingora's thread-safety bounds.
Type mismatches return None instead of corrupting or removing the stored entry. remove_filter_state() also preserves the entry on type mismatch.
3. Identity Set/Clear Around Filter Hooks
The HTTP pipeline executor sets current_filter_id before invoking a filter hook and clears it immediately afterward.
This is covered for:
The important detail is cleanup ordering. The executor stores the hook result first, clears current_filter_id, and only then propagates errors or returns rejections. That avoids leaking a stale filter identity into later pipeline work.
When no filter hook is actively running, current_filter_id is None. Calls like
get_filter_state<T>()then return None, andinsert_filter_state<T>()is a no-op. This keeps state access scoped to the currently executing filter.4. Pingora Request Context State Persistence
PingoraRequestCtx now owns the durable per-request filter_state map between filter phases.
HttpFilterContext is temporary: the protocol layer builds one for a specific hook execution, runs the filter pipeline, and then drops it. Without a durable owner outside that temporary context, state written during request handling would disappear before request-body or response handling.
The protocol layer now swaps filter_state into HttpFilterContext before filter execution and writes it back afterward, following the existing filter_metadata ownership pattern. This keeps state owned by exactly one context at a time and avoids shared references or locks.
The Pingora handler changes are intentionally narrow. They do not change request routing, body buffering, response handling, retry behavior, or upstream selection. They only extend the existing context move/writeback boundary so typed filter state survives across lifecycle callbacks.
5. Pipeline Pinning Across Hot Reload
PingoraRequestCtxnow pins the currentArc<FilterPipeline>during request_filter.Later hooks use the pinned pipeline instead of loading from ArcSwap again. The helper is idempotent, so a second call does not repin after a reload.
This matters because filter_state is keyed by filter_id, and filter_id is only meaningful within the pipeline that assigned it. If a request started on pipeline A, then a hot reload swapped in pipeline B before the response phase, the same numeric filter_id could refer to a different filter instance.
Pinning prevents that. A request keeps using the pipeline it started with for the full request lifecycle.
This guarantees:
More on Pingora Handler Changes
The Pingora handler changes are intentionally narrow. They do not change request routing, body buffering, response handling, retry behavior, or upstream selection.
They do two things:
Carry filter state across lifecycle callbacks.
Each handler phase already builds a short-lived
HttpFilterContextfromPingoraRequestCtx, runs the filter pipeline, then writes durable fields back toPingoraRequestCtx.This PR adds
filter_stateto that existing move/writeback pattern, matching howfilter_metadatais already handled. The state map is moved into the filter context withstd::mem::take()and moved back after the hook returns.Use one pinned pipeline for the full request lifecycle.
request_filterpins the currentArc<FilterPipeline>fromArcSwapintoPingoraRequestCtx.Later Pingora callbacks call
ctx.pipeline(&self.pipeline)instead of loading fromArcSwapdirectly. If the request was pinned, this returns the same pipeline. If a callback runs before pinning in an early-failure path, it falls back to loading the current pipeline.This means hot reload remains safe without changing the behavior of existing filters: active requests keep using the pipeline they started with, and new requests pick up the reloaded pipeline.
PR Stack Context
HttpFilterContext, stable filter identity, pipeline pinning across hot reloadExtProcExchangetransport core for one persistent bidirectional gRPCExternalProcessor.Processstream per requestext_procinto the filter lifecycle for the llm-d Go EPP request-routing path using genericext_proc+endpoint_selectorWhy This Is Needed
The upcoming full-duplex ext_proc integration needs to open a processor exchange during one phase and continue using that same exchange in later phases.
Without this PR, filters do not have a safe request-scoped place to store typed state across hooks. Using globals, static maps, or ad hoc protocol-layer fields would make ownership, cleanup, hot reload, and concurrency harder to reason about.
This PR keeps the foundation generic:
Validation