Merge sessions map and pruning logic #224
Merged
Conversation
This file contains 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
Currently sessions management logic is mixed in with the actual server so this moves it out to a separate SessionManager. I originally wanted to re-use this SessionManager to add session affinity to the load balancer but quickly realised that this wouldn't be possible currently because looking up a session is unfortunately async (due to locking) and filters aren't async. Not currently sure how we'd want to tackle this issue, atm at least would rather not do an async Filter trait since that brings other issues and we might end up with a perf regression spawning a new task per packet
|
Not specific to this PR, but:
Sounds like we should tackle a project to remove as much of the async locking as possible. (Maybe after we have a performance testbed?) |
markmandel
reviewed
Apr 13, 2021
markmandel
reviewed
Apr 13, 2021
| @@ -357,7 +351,7 @@ mod tests { | |||
| .unwrap() | |||
| .as_secs(); | |||
| let diff = initial_expiration_secs - now_secs; | |||
| assert!(diff >= 15 && diff <= 20); | |||
| assert!((15..21).contains(&diff)); | |||
markmandel
approved these changes
Apr 13, 2021
Co-authored-by: Mark Mandel <markmandel@google.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.
Currently sessions management logic is mixed in with the actual server
so this moves it out to a separate SessionManager.
I originally wanted to re-use this SessionManager to add
session affinity to the load balancer but quickly realised that this
wouldn't be possible currently because looking up a session is
unfortunately async (due to locking) and filters aren't async. Not
currently sure how we'd want to tackle this issue, atm at least would rather not
do an async Filter trait since that brings other issues and we might
end up with a perf regression spawning a new task per packet