Session affinity scorer #117
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmitripikus
commented
May 4, 2025
- Session affinity scorer implementation
- Fix in request headers transfer
* dev: Added a simple unit test for the PostResponse plugin invocation Invoke the PostResponse handlers and send any added headers to the user Added code to scheduler to enable running the PostResponse plugins Use an init() function instead of modifying the scheduler code to inject our config Added PostResponse to scheduler config Fixed scorer tests
vMaroon
reviewed
May 4, 2025
| } | ||
|
|
||
| func (s *SessionAffinityScorer) Name() string { | ||
| return "session affinity scorer" |
There was a problem hiding this comment.
s/session affinity scorer/session-affinity-scorer/ for alignment with other scorers.
vMaroon
reviewed
May 4, 2025
| // session was sent to, by giving that pod the specified weight and assigning | ||
| // zero score to the rest of the targets | ||
| type SessionAffinityScorer struct { | ||
| sessions *sync.Map |
nirrozenbaum
reviewed
May 4, 2025
| @@ -74,3 +74,8 @@ type PostResponse interface { | |||
| Plugin | |||
| PostResponse(ctx *types.SchedulingContext, pod types.Pod) | |||
| } | |||
|
|
|||
| type ScorerWithPostResponse struct { | |||
There was a problem hiding this comment.
this struct is redundant
nirrozenbaum
reviewed
May 4, 2025
| sessions: &sync.Map{}, | ||
| } | ||
| return plugins.ScorerWithPostResponse{ | ||
| Scorer: s, |
There was a problem hiding this comment.
if you remove the struct from prev comment, then here just return s
nirrozenbaum
reviewed
May 4, 2025
| reqHeaders := ctx.Req.Headers | ||
|
|
||
| var sessionId = "" | ||
| for k, v := range reqHeaders { |
There was a problem hiding this comment.
this foor loop is very inefficient. headers field is a map (hash), means we can check if key exists in O(1) without iteration over all keys
nirrozenbaum
reviewed
May 4, 2025
|
|
||
| func (s *SessionAffinityScorer) PostResponse(ctx *types.SchedulingContext, pod types.Pod) { | ||
| ctx.MutatedHeaders[sessionIDHeader] = encode(pod.GetPod().NamespacedName.String()) | ||
| } |
There was a problem hiding this comment.
is this a temp hack?
it puts as session id the namespaced name of the pod (encoded).
meaning different requests that are served by the same pod get the same value, which is not unique (not ID)
There was a problem hiding this comment.
Right, it's not a session Id, but a session token. I'll rename the header to Session-Token
- Map fetch is done instead of loop
nirrozenbaum
approved these changes
May 5, 2025
Closed
clubanderson
pushed a commit
that referenced
this pull request
May 7, 2025
* 'session affinity scorer' partial implementation (without headers in response) * Fix in filling request headers * Encoded value of namespaced pod name is sent in response to client * Support of session affinity scorer configuration via environment variables, is added * Go file for session affinity scorer is renamed * Redundant 'sessions' field is removed * Redundant 'ScorerWithPostResponse' struct is removed * - SessionID is renamed to sessionToken - Map fetch is done instead of loop * Session token name is changed to 'x-session-token' * Minor fixes are made in README * Small fix after merge --------- Co-authored-by: Shmuel Kallner <kallner@il.ibm.com>
clubanderson
pushed a commit
that referenced
this pull request
May 7, 2025
* 'session affinity scorer' partial implementation (without headers in response) * Fix in filling request headers * Encoded value of namespaced pod name is sent in response to client * Support of session affinity scorer configuration via environment variables, is added * Go file for session affinity scorer is renamed * Redundant 'sessions' field is removed * Redundant 'ScorerWithPostResponse' struct is removed * - SessionID is renamed to sessionToken - Map fetch is done instead of loop * Session token name is changed to 'x-session-token' * Minor fixes are made in README * Small fix after merge --------- Co-authored-by: Shmuel Kallner <kallner@il.ibm.com>
clubanderson
pushed a commit
that referenced
this pull request
May 7, 2025
* 'session affinity scorer' partial implementation (without headers in response) * Fix in filling request headers * Encoded value of namespaced pod name is sent in response to client * Support of session affinity scorer configuration via environment variables, is added * Go file for session affinity scorer is renamed * Redundant 'sessions' field is removed * Redundant 'ScorerWithPostResponse' struct is removed * - SessionID is renamed to sessionToken - Map fetch is done instead of loop * Session token name is changed to 'x-session-token' * Minor fixes are made in README * Small fix after merge --------- Co-authored-by: Shmuel Kallner <kallner@il.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.