-
Notifications
You must be signed in to change notification settings - Fork 509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GEP-1619: Add context on session persistence naming #2904
GEP-1619: Add context on session persistence naming #2904
Conversation
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding all the extra context.
/ok-to-test
/cc @youngnick @robscott
geps/gep-1619/index.md
Outdated
One concern for using the name "session persistence" is that it may confuse the idea of persisting a session to storage. | ||
This confusion is particularly common among Java developers, as Java defines session persistence as storing a session to | ||
disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One concern for using the name "session persistence" is that it may confuse the idea of persisting a session to storage. | |
This confusion is particularly common among Java developers, as Java defines session persistence as storing a session to | |
disk. | |
> **Note**: One concern for using the name "session persistence" is that it may confuse the idea of persisting a session to storage. | |
> This confusion is particularly common among Java developers, as Java defines session persistence as storing a session to disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
This is a great update, nice work @gcs278. Happy to LGTM as-is once it's move out of draft. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gcs278!
/approve
geps/gep-1619/index.md
Outdated
**Accepted definitions of "strong session affinity":** | ||
|
||
- Session Persistence (preferred) | ||
- Sticky Sessions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure it's helpful or useful to have multiple ways to refer to this concept in Gateway API. I think it may be useful to have docs that mention some common alternative names for what we refer to as "Session Persistence", but I'd rather just settle on one name for this concept within Gateway API and run with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea that's a good point. Better to keep it simple with less options for folks to remember. Removed.
and weak session affinity (what this GEP calls session affinity) which we will define further in [The Relationship of Session Persistence and Session Affinity](#the-relationship-of-session-persistence-and-session-affinity). | ||
In this context, "strong" implies a guarantee, while "weak" indicates a best-effort approach. | ||
|
||
Here's a survey of how some implementations refers to these ideas: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: May want to include Envoy itself here, which uses "Stateful Session" terminology https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/stateful_session_filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. But they also refer to it as "session stickiness". I'll put both.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, robscott, shaneutt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Add a section "Naming" which outlines our decision for choosing session persistence and session affinity. Update section about session affinity to include the fact that session affinity CAN use cookies or headers. Also add details about the difference between persistence and affinity across proxy restarts or backend pool change.
0488561
to
43b93be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and weak session affinity (what this GEP calls session affinity) which we will define further in [The Relationship of Session Persistence and Session Affinity](#the-relationship-of-session-persistence-and-session-affinity). | ||
In this context, "strong" implies a guarantee, while "weak" indicates a best-effort approach. | ||
|
||
Here's a survey of how some implementations refers to these ideas: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. But they also refer to it as "session stickiness". I'll put both.
geps/gep-1619/index.md
Outdated
**Accepted definitions of "strong session affinity":** | ||
|
||
- Session Persistence (preferred) | ||
- Sticky Sessions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea that's a good point. Better to keep it simple with less options for folks to remember. Removed.
geps/gep-1619/index.md
Outdated
One concern for using the name "session persistence" is that it may confuse the idea of persisting a session to storage. | ||
This confusion is particularly common among Java developers, as Java defines session persistence as storing a session to | ||
disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
/lgtm |
What type of PR is this?
/kind gep
What this PR does / why we need it:
Add a section "Naming" which outlines our decision for choosing session persistence and session affinity. Also recognize sticky session as a valid alternative name.
Update section about session affinity to include the fact that session affinity CAN use cookies or headers. Also add details about the difference between persistence and affinity across proxy restarts or backend pool change.
Discussion: #2893
Which issue(s) this PR fixes:
Fixes #
Updates for #1619
Does this PR introduce a user-facing change?: