migrates weighted loadbalance strategy to user new RequestInterceptors for Stats tracking#946
Merged
OlegDokuka merged 2 commits intomasterfrom Oct 22, 2020
Merged
Conversation
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
rstoyanchev
reviewed
Oct 22, 2020
Contributor
rstoyanchev
left a comment
There was a problem hiding this comment.
Looks great overall.
One consequence is that WeightedLoadBalanceStrategy can no longer be instantiated directly for the purpose of being used with LoadbalanceRSocketClient. I think WeightedLoadbalanceStrategy should be an InitializingLoadbalanceStrategy but the management of that Map as well as the logic to initialize the connector could be in an inner class (rather than a sub-class) which implements Function<RSocket, StatsRequestInterceptor>. So then if WeightedLoadbalanceStrategy is created without an externally provided Function it falls back on an instance of that inner class.
rsocket-core/src/main/java/io/rsocket/loadbalance/StatsRequestInterceptor.java
Outdated
Show resolved
Hide resolved
rsocket-core/src/main/java/io/rsocket/loadbalance/LoadbalanceRSocketClient.java
Outdated
Show resolved
Hide resolved
spencergibb
approved these changes
Oct 22, 2020
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
bf34b21 to
626a9f5
Compare
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
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 PR refactors current weighted rsocket to another mechanism which makes Stats to be hidden from the outside world and make it accessible only for WeighetedLoadbalanceStrategy.
This PR introduces StatsRequestInterceptor, which is a public class, hence can be used not only for Client-side but as well as for the server one (e.g. broker case).
The WeightedLoadbalanceStrategy, gets a new constructor API which lets one using selected RSocket to extract associated StatsRequestInterceptor and then calculate the weight.