confluentinc / confluentinc/confluent-sql

[optimization] Proactive refresh daemon

Open
#157 0 comments 0 reactions 1 assignee Claimed by @jlrobins View on GitHub
Dominant language
Python
Stars
6
Forks
1
Avg merge
21h 35m
Merged PRs (30d)
27

Description

- **Goal:** keep the ~4-round-trip refresh latency off the hot path during an *actively-querying*
session by refreshing both tokens in the background ahead of the CP token's ~5-min expiry. This is
a pure latency optimization — correctness already lives in child 3's on-request refresh — so it
lands **after** a functional PoC, not as a table-stakes hurdle before one.
- **Opt-in, never default.** Child 3's synchronous on-request refresh is already functional, so the
daemon (and the whole park/refcount lifecycle below) must stay *dormant* unless a caller explicitly
asks for it. A default `connect(auth="oauth")` stays byte-for-byte child-3 behavior: no thread, no
park lifecycle, synchronous on-request refresh. The complexity here switches on only for the opt-in
caller.
- **Activation is a new field on the `OAuthPolicy` struct introduced in #198**, *not* a new
`connect()` kwarg and *not* a new `auth=` value. #198 replaces #156's flat `reauth` scalar with a
frozen `OAuthPolicy` dataclass (passed as `connect(oauth_policy=...)`) precisely so this child
adds a *field*, not another look-alike `oauth_*` parameter. #198 named its field
`on_reauthentication_required` (rather than a bare `reauth`) specifically so a `refresh`-shaped
field here wouldn't read as a near-duplicate of it; this child follows the same instinct and
spells its own field `token_refresh_mode` rather than a bare `refresh`. This child adds:
```python
@dataclass(frozen=True)
class OAuthPolicy:
on_reauthentication_required: Literal["auto", "raise"] = "auto" # #198
token_refresh_mode: Literal["on_demand", "background"] = "on_demand" # this child
```
`"on_demand"` = child-3 behavior; `"background"` = run this daemon. Plus an exported preset naming
the intent, e.g. `OAUTH_LOW_LATENCY = OAuthPolicy(on_reauthentication_required="auto",
token_refresh_mode="background")`, alongside #198's `OAUTH_INTERACTIVE` / `OAUTH_UNATTENDED`.

A fourth cell this shape allows but names no preset for -- `OAuthPolicy(on_reauthentication_required="raise",
token_refresh_mode="background")` -- is real, not degenerate: a long-running unattended job that
wants the daemon's latency benefit while the session is alive, but wants the eventual 8h-wall
death to surface as an immediate exception on the next query rather than a doomed browser-login
attempt that blocks for a full login timeout before failing anyway. Left unnamed deliberately;
construct it directly if needed.
- **Cross-connection disagreement is a hard `InterfaceError` — and the guard already exists.** #198
makes the holder's one-identity guard compare the whole frozen `OAuthPolicy` ("one process = one
OAuth identity **and one policy**"): a second connection whose *explicit* `oauth_policy` differs
from the established login is refused, exactly as for environment and org; an omitted (`None`)
policy inherits and never conflicts. So this child inherits that behavior for free the moment
`token_refresh_mode` becomes a field — no new guard clause, no per-field sentinel. Multi-connection
is only expected via a factory minting *identical* connections, so the guard is a misconfig
backstop that essentially never fires in practice.
- **Scope (in):**
- the `token_refresh_mode` field on `OAuthPolicy` + its `OAUTH_LOW_LATENCY` preset (per above);
validation is already handled by #198's `oauth_policy` guard (rejecting a non-`None` policy when
`auth != "oauth"`), so no new `_resolve_oauth_config` clause is needed here.
- thread the resolved `policy.token_refresh_mode == "background"` through `oauth.acquire()` (as a
`background_refresh: bool`); the holder starts the daemon on the shared provider when requested.
`OAuthProvider` gains an idempotent `start_background_refresh()` + stop, and `close()`/`shutdown()`
stop the daemon.
- a background **daemon thread** that sleeps on a `threading.Event` until `cp_expires_at − window`
(the CP token is the shorter clock), wakes, and runs `_refresh()` through the *same* single-flight
gate as the on-request path; the `Event` lets `close()` interrupt it promptly.
- the holder-side lifecycle the daemon necessitates: **refcount** of open OAuth Connections,
**park-don't-evict** at refcount 0 (stop the daemon, keep the tokens — reversible), a short
cancel-on-acquire **linger** to absorb dbt churn, `Connection.close()` → `holder.release()`
gaining teeth, and `daemon=True` as the never-hang backstop. Because the daemon is opt-in, at
refcount 0 with no connection having requested `background` there is simply nothing to park — the
holder falls back to child-4's keep-alive-until-`shutdown_all()`, so this lifecycle only engages
for the opt-in path.
- **Out of scope:** the on-request refresh (child 3, still the correctness floor — the daemon only
pre-empts its latency); the `OAuthPolicy` struct, presets, and whole-policy guard themselves (#198,
which this child only extends by one field); proactive re-login ahead of the 8h wall (child 8, which
rides this daemon).
- **Depends on:** #198 for the frozen `OAuthPolicy` struct itself (one field,
`on_reauthentication_required`, no holder/guard changes) -- this child is what adds the second
field *and* the whole-policy cross-connection guard on top of it; neither exists yet as of #198.
Through #198, also children #153, #154.
- **Prior art:** *ide-sidecar* — `RefreshCCloudTokensBean` (`@Scheduled refreshTokens()` every 60s,
`ConcurrentExecution.SKIP`) with the proactive decision `shouldAttemptTokenRefresh()` (refresh iff
a token expires before the next tick). *mcp-confluent* — `AuthContext.startRefreshLoop()` /
`scheduleNextRefresh()` (`setTimeout` at `controlPlaneExpiresAt − window`). **Neither borrows the
refcount / park-don't-evict / linger lifecycle, nor an opt-in toggle** — both simply refresh for
the whole process / connection lifetime; that lifecycle, and keeping it opt-in, is our own, driven
by dbt's open-close-reopen churn and by child 3 already being a sufficient correctness floor.
- **Sized right:** one thread + the refcount/park lifecycle + one `OAuthPolicy` field & preset +
concurrency tests, layered on an already-shipped provider/holder and #198's already-guarded policy
surface.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.