-
Notifications
You must be signed in to change notification settings - Fork 7
Session affinity scorer #52
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
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Maya Barnea <mayab@il.ibm.com>
Signed-off-by: Maya Barnea <mayab@il.ibm.com>
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.
This file seems like it belongs to a different PR and not to a session affinity scorer
isFirst := true | ||
|
||
for _, pod := range pods { | ||
if isFirst { |
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.
There is no reason to test isFirst
on every iteration, by doing some work outside the loop:
- set the first pod and its score as the maximum Pod and score
- set
maxScore = -math.MaxFloat64
ctx.Logger.V(logutil.DEBUG).Info(fmt.Sprintf("Selecting a pod with maximum score from %d candidates: %+v", len(pods), pods)) | ||
|
||
// select pod with maximum score, if more than one with the max score - use random pods from the list | ||
var highestScoreTargets []types.Pod |
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: not sure if it makes sense to have such long variable names in a relatively short function. I don't think we have a style guide, but Go seems to favor shorter names in smaller scopes
Session affinity scorer uses information stored in datastore (map of session id to pod) to set score=1 to pod for the given session id (if exists) or 0 to all other pods