Skip to content
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

ISICO-14127: roundrobin requests to dynomite nodes #156

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

charybr
Copy link

@charybr charybr commented Feb 9, 2023

Problem:
In our case, all nodes in dynomite cluster are replicas without any sharding. So we are using same Token for all 3 nodes. By default load balancing strategy used by Dyno client is TokenAware. Also local rack affinity(availabilityZone) set to us-east-1a. As a result all the requests will go to one node that exists in us-east-1a.

Solution:
Configure Dyno client to use RoundRobin strategy. But In our topology we have only one node per rack and availabilityZone(localRack) set to a rack. Even after setting LBStrategy to Roundrobin, Dyno client will try to distribute within the localRack, which results in all requests going to that one node in that specific rack.

Given the above topology, for roundrobin LB strategy to work we need to set availabilityZone to null. So that there is no localRack affinity and requests get distributed.

Raghavendra Chary B added 2 commits January 4, 2023 12:21
…e need

to set availabilityZone to null. So that there is no localRack affinity
and requests get distributed.
@charybr charybr requested review from manjurad and sarmuru2 February 9, 2023 15:08
Copy link

@sarmuru2 sarmuru2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont need to specifically set LocalRack to null. please check it

@charybr
Copy link
Author

charybr commented Feb 10, 2023

we dont need to specifically set LocalRack to null. please check it

We could have removed the line setLocalRack. Retained it to highlight that we don't need local rack affinity.

Copy link
Collaborator

@manjurad manjurad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't merge any changes to master directly. Open the change to integ/commonbranch and after testing is done in QA cloud, only then should changes be merged into master

* So that there is no localRack affinity and requests get distributed.
*/
.setLoadBalancingStrategy(LoadBalancingStrategy.RoundRobin)
.setLocalRack(null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of making this change in code, isn't it better to remove this setting from the config
https://bitbucket-eng-sjc1.cisco.com/bitbucket/projects/AN/repos/galaxy/browse/docker/config/dynomite-cluster.config.properties#78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants