enhance(load balance): add StickySession load balance strategy #673
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
ibigbug
reviewed
Jan 16, 2025
| @@ -63,3 +80,195 @@ pub fn strategy_consistent_hashring() -> StrategyFn { | |||
| ))) | |||
| }) | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
There was a problem hiding this comment.
can we move this into mod tests
There was a problem hiding this comment.
TEST_LRU_STATE is coupled with the strategy fn, so i guess not.
for better readability, the #[cfg(test)] lines can be removed, but then there is basically no way to perform the unittest for the lru_cache state inside the closure.
ibigbug
reviewed
Jan 16, 2025
| index = jump_hash(key + timestamp(), buckets) as usize; | ||
| } | ||
| // TODO: if we should just remove the key from the cache? | ||
| lru_cache_clone.lock().await.insert(key, 0); |
There was a problem hiding this comment.
if no proxy is alive maybe we can just remove the entry and build the cache next time?
There was a problem hiding this comment.
i think so, the current solution - use the first entry of proxies as fallback - is written by clash.meta.
it's basically ok, since the cached proxy will be checked before chosen as the return result
ibigbug
approved these changes
Jan 16, 2025
lakako
pushed a commit
to lakako/clash-rs
that referenced
this pull request
Jan 18, 2025
…q#673) * feat: add StickySession load balance strategy * move noopxxx to test_utils
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.
🤔 This is a ...
🔗 Related issue link
enhancement in (#563)
💡 Background and solution
refer to clash.meta's solution
strategy details:
tests:
NoopResolverandNoopOutboundHandlerto create ProxyManager freely (shall i separate it into another PR?)📝 Changelog
enhance(load balance): support sticky session strategy.
☑️ Self-Check before Merge
unitests in
loadbalance/helpers.rs