Skip to content

docs: propose pipeline-scoped filter task supervisor - #7

Open
aslakknutsen wants to merge 3 commits into
praxis-proxy:mainfrom
aslakknutsen:background_tasks
Open

docs: propose pipeline-scoped filter task supervisor#7
aslakknutsen wants to merge 3 commits into
praxis-proxy:mainfrom
aslakknutsen:background_tasks

Conversation

@aslakknutsen

@aslakknutsen aslakknutsen commented Aug 27, 2026

Copy link
Copy Markdown

What does this PR do?

Filters have no lifecycle for work that must die with the pipeline, so overlay reload invented a private thread and azure_ad is copying it. Track the What/Why against praxis#1042.

Discussion link

Checklist

  • Discussion link is in proposal frontmatter
  • Issue link is in proposal frontmatter
  • Authors and stakeholders listed
  • Repos list includes all affected repositories
  • Graduation criteria are specific and verifiable
  • First PR contains only What? and Why? (no How?)

Filters have no lifecycle for work that must die with the
pipeline, so overlay reload invented a private thread and
azure_ad is copying it. Track the What/Why against praxis#1042.

Signed-off-by: Aslak Knutsen <aslak@4fs.no>
@alexsnaps

Copy link
Copy Markdown
Member

I very much like the problem statement here. I'm wondering if a "tighter" lifecycle & matching callback couldn't also "help with that". Not to invalidate or rescope this tho, just food for thoughts.

Also, most (all?) of the current use cases are effectively "refresh ahead caching use cases" - having a centralized location to coordinate across multiple refreshes might be desirable to avoid "thundering herd" problems, less on the SoR in this case, but rather in our client (e.g. serialize calls beyond n-so many concurrent on-going ones). Some initial thoughts on the subject - I'm obviously tainted by years of working in the caching business...

@aslakknutsen

aslakknutsen commented Aug 27, 2026

Copy link
Copy Markdown
Author

I agree the cash refresh usecase specifically could be a good target as well and might build on a common foundation with tasks. I have a feeling a cache refresh would be tied to a Cache and somehow related to the Store.

@szedan-rh

Copy link
Copy Markdown

Thanks for writing this up matches what I've been sketching for #555/#810 on the credential-store side, and it's good to see we landed on the same process-vs-pipeline-lifetime boundary independently.

Two things worth pinning down in the "How?" phase:

  1. Fail construction vs. fail closed you've already flagged this as an open graduation criterion, want to +1 it explicitly: credential refresh (Entra/GCP ADC) needs fail closed (pipeline builds fine, requests 503 until first successful fetch), not fail construction like overlay wants. Worth confirming the supervisor API supports both from day one rather than defaulting to one.
  2. Credential refresh can't be pipeline-scoped for cancellation, only for "runtime now exists." A refresh task tied to this pipeline's cancellation would get killed and respawned on every hot reload, re-authenticating against the IdP on every config touch, which is the exact waste #555 is trying to avoid if we want to dedupe shared credentials across filter instances. The pipeline-build hook is the right place to know a runtime is available and to register work, but the credential store needs its own process-lifetime cancellation, independent of any one pipeline's drop. Might be worth calling that out explicitly so a naive "How?" implementation doesn't wire refresher lifetime 1:1 to pipeline lifetime.

@shaneutt shaneutt self-assigned this Aug 28, 2026
@shaneutt shaneutt moved this to Review in Core Proxy Aug 28, 2026

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary: The proposal clearly motivates the need for pipeline-scoped background task supervision and correctly identifies the problem with the current thread-per-filter pattern. However, critical design questions about the API surface, failure modes, and task lifecycle are left too vague for stakeholders to assess feasibility even at the What/Why stage.

Severity Count
Large 2
Medium 3

Comment thread proposals/01042_filter-task-supervisor.md
Comment thread proposals/01042_filter-task-supervisor.md
Comment thread proposals/01042_filter-task-supervisor.md
@alexsnaps

Copy link
Copy Markdown
Member

@szedan-rh on "Credential refresh" - I do think the tasks themselves should be tied to the pipeline. Now, I'm wondering whether the data stored by a filter shouldn't have a slight different lifecycle than the pipeline because of how we currently reload. If we'd know what data belongs to what filter ("statically owned"), we could GC data only when not needed after a reload, as the filter is gone.

@praxis-proxy praxis-proxy deleted a comment from praxis-bot Aug 31, 2026
@praxis-proxy praxis-proxy deleted a comment from praxis-bot Aug 31, 2026

@shaneutt shaneutt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments I would like to see resolved, and then LGTM for initial pass. I understand, and agree with what we want to do and why.

Comment thread proposals/01042_filter-task-supervisor.md Outdated
Comment thread proposals/01042_filter-task-supervisor.md Outdated
Comment on lines +193 to +207
- As a filter author, I want to start
pipeline-scoped background work without creating
a thread or runtime so that credential refresh
and file watch stay in-tree without copying
overlay internals.
- As a filter author, I want spawn failure to fail
pipeline build so that a dead refresher cannot
ship as a live 503-forever filter.
- As an operator, I want hot reload to stop the old
pipeline's background work so that refreshers and
watchers do not accumulate across config swaps.
- As a maintainer, I want one place that owns
filter background lifetime so that AI, core, and
extproc do not grow independent thread+runtime
copies.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

aslakknutsen and others added 2 commits September 1, 2026 15:13
Co-authored-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Aslak Knutsen <aslak.tux@gmail.com>
Co-authored-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Aslak Knutsen <aslak.tux@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

5 participants