-
Notifications
You must be signed in to change notification settings - Fork 132
fix: avoid unbalanced session pool creation #2442
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
bb09e1b
fix: avoid unbalanced session pool creation
olavloite c2ef5b1
fix: automatically balance pool
olavloite 8a44f2b
fix: skip empty pool
olavloite 75773fd
fix: shuffle if unbalanced
olavloite 913fa8a
fix: only reset randomness if actually randomized
olavloite 4130c8c
test: randomize if many sessions are checked out
olavloite 6c48990
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] f8637bb
test: try with more channels
olavloite be9fca2
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 9be0abe
Merge branch 'main' into randomize-session-position-at-first-add
olavloite f260eca
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 8939c7e
fix: also consider checked out sessions for unbalanced pool
olavloite 80ded1b
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 8a84488
Merge branch 'main' into randomize-session-position-at-first-add
olavloite 8346c5c
docs: add javadoc for property
olavloite 7a6e536
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 5e3d68d
perf: optimize low-QPS workloads
olavloite ed25c99
test: only randomize if more than 2 sessions are checked out
olavloite bc406f4
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 104b36e
test: only skip randomization for existing sessions
olavloite f5095f3
Merge branch 'randomize-session-position-at-first-add' of github.com:…
olavloite 6885745
Merge branch 'main' into randomize-session-position-at-first-add
olavloite 86909e3
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] 8e7ea3a
chore: run formatter
olavloite c6b1542
Merge branch 'randomize-session-position-at-first-add' of github.com:…
olavloite 30d7edc
chore: address review comments
olavloite 51320ab
Merge branch 'main' into randomize-session-position-at-first-add
olavloite 3c6060a
docs: update comment on how session is added to the pool
olavloite File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
perf: optimize low-QPS workloads
- Loading branch information
commit 5e3d68dfa5d9438a4c04cdabac5234bc70ce4ebc
There are no files selected for viewing
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
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.
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.
Can we add
@VisibleForTesting
and add a few more unit tests for this method? It's doing a bunch of things which is currently untested with the existing tests.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.
Yeah, good point. I've changed it a bit so we have a static method that does the actual calculation. That method is easier to test than one that depends on all the state fields in the SessionPool class. And I've added different tests for that method.