docs: add sticky sessions proposal - #904
Conversation
|
Proposal validation failed:
|
abdallahsamabd
changed the title
07a95d4 to
3a86fa6
Compare
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
Review Summary
Good proposal that clearly defines the problem space and motivates the feature well. The What/Why structure follows the template, the user stories are concrete, and the consistent-hashing limitations are well-enumerated.
The main gap is the relationship to proposal #99 (Stateful Proxy State Management), which explicitly lists sticky sessions as a feature driver and defines the storage patterns this proposal's shared mapping would need. The proposal should also address bounded-state requirements given the project's conventions around memory safety.
| Severity | Count |
|---|---|
| Large | 2 |
| Medium | 2 |
Automated review by praxis-bot
| response (e.g. Set-Cookie: JSESSIONID) and | ||
| automatically build session-to-endpoint mapping | ||
| - Shared session-to-endpoint mapping: thread-safe, | ||
| cross-worker data structure |
There was a problem hiding this comment.
[Large] The shared session-to-endpoint mapping is exactly the "shared hot-path state" pattern defined in proposal #99 (Stateful Proxy State Management). That proposal is in proposed status and explicitly lists sticky sessions in its Feature Drivers table with the direction: "Typed session and task stores with TTLs and config-generation awareness."
Add a cross-reference to proposal #99 and clarify the relationship: does this proposal depend on #99's state model being accepted first, or does it intend to define its own storage approach independently? Without that context a reviewer cannot evaluate whether the shared mapping design will be consistent with the project-wide state strategy.
There was a problem hiding this comment.
Added cross-reference to proposal #99 in the feature list (lines 41-45) and a dependency statement in Goals (lines 80-84) clarifying that this proposal depends on #99's state model for the storage layer
| Express) work without modification. | ||
| - Provide a shared session-to-endpoint mapping visible | ||
| to all proxy workers so routing is consistent | ||
| regardless of which worker handles the request. |
There was a problem hiding this comment.
[Large] The goals mention a shared mapping "visible to all proxy workers" but omit bounded-state requirements. The project's existing patterns require bounded state with explicit constraints: the per-IP rate limiter uses a 100K soft / 200K hard entry cap, and proposal #99 requires that "all hot-path state calls to have bounded timeouts and visible metrics."
Add goals for:
- TTL or max-age on session-to-endpoint mappings (what happens to stale entries?)
- Entry count bounds or eviction policy (what prevents unbounded memory growth under high session cardinality?)
- Behavior on hot-reload (proposal #99 notes that pipeline reload resets local filter state -- session mappings would be lost if filter-owned)
These are What-level constraints, not How-level details, because they affect whether the feature is viable without external storage.
There was a problem hiding this comment.
Added bounded-state goals: TTL-based expiry, configurable entry count cap with eviction policy, visible metrics, and explicit hot-reload behavior (lines 70-84)
| 4. **No failover** — if the hashed endpoint is | ||
| unhealthy, the ring probes adjacent slots rather | ||
| than maintaining a stable re-pin for the session | ||
| lifetime. |
There was a problem hiding this comment.
[Medium] "No failover" is slightly misleading. The consistent-hash implementation does handle unhealthy endpoints: it probes adjacent ring slots and falls back to the hashed slot only when all endpoints are unhealthy (see ConsistentHash::select in filter/src/load_balancing/consistent_hash.rs).
The actual limitation is that the fallback is ephemeral and unstable: the probed-to endpoint is not persisted as a binding, so if the original endpoint recovers, the session silently moves back; and if topology changes, the probe path itself shifts. Suggest rephrasing to: "No stable failover -- when the hashed endpoint is unhealthy, the ring probes adjacent slots but does not persist the new binding, so sessions can bounce between endpoints across health transitions and topology changes."
There was a problem hiding this comment.
Rephrased to "No stable failover" acknowledges the ring probing but clarifies that the binding is not persisted, causing sessions to bounce across health transitions and topology changes.
| --- | ||
| issue: https://github.com/praxis-proxy/praxis/issues/108 | ||
| discussion: | ||
| - https://github.com/orgs/praxis-proxy/discussions/905 |
There was a problem hiding this comment.
[Medium] The discussion field uses a YAML list format, but the proposal template (docs/proposals/template.md) and existing proposals (e.g., #99) use a scalar value. Change to scalar format for consistency:
discussion: https://github.com/orgs/praxis-proxy/discussions/905There was a problem hiding this comment.
Fixed. Changed from YAML list to scalar format.
d54e12e to
a712d5a
Compare
|
Unsigned commits: a712d5a. Please sign your commits. |
a712d5a to
b83163d
Compare
Signed-off-by: Abdallah Samara <abdallahsamabd@gmail.com>
b83163d to
ba7cba8
Compare
3 participants
What does this PR do?
Adds the What/Why proposal for Sticky Sessions / Session Affinity. This defines the problem space (lack of cookie-based persistence, no learn mode, no shared session mapping, no graceful failover) and motivates the feature with user stories covering browser clients, legacy apps, API clients, and multi-worker deployments.
Which issue(s) does this relate to?
Fixes #108
Checklist
git commit -s)make lint && make testpasses locallyDoes this introduce a breaking change?
No. This is a proposal document only — no code changes.