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

Simplify tokenAwareHostPolicy.Pick #197

Merged

Conversation

dkropachev
Copy link
Collaborator

@dkropachev dkropachev commented Jun 22, 2024

Main goal of this PR is to make tokenAwareHostPolicy.Pick code less convoluted.
But there are little functional changes:

Start shuffling hosts obtained from tablets

Before this PR hosts from tablets are not being shuffled

Consider next hosts information source if current got 0 hosts

There are three host information sources: tablets, meta and tokenring,
Before this PR if tables returned no hosts for given token, in certain cases code did not consider meta or tokenring.
And same for meta, it did not consider tokenring.
After this PR, it starts goes through tablets, meta and tokenring, if current host sources yield no hosts it proceed to next one.

Removes unnecessary locks

There was locking done on cowTabletList at t.tablets to read tablets, but since cowTabletList solves parallel access issue through COW, it is not needed.

@mykaul
Copy link

mykaul commented Jun 22, 2024

What issues does it solve? A one-line description (at least) would be appreciated.

@dkropachev
Copy link
Collaborator Author

What issues does it solve? A one-line description (at least) would be appreciated.

Added

@dkropachev dkropachev force-pushed the dk/simlify-tokenaware-policy-pick branch from 598a4ae to ce30161 Compare June 24, 2024 11:34
@sylwiaszunejko
Copy link
Collaborator

Before this PR hosts from are not being shuffled

from tokenring? There is a missing information in the commit message and PR description

@dkropachev
Copy link
Collaborator Author

Before this PR hosts from are not being shuffled

from tokenring? There is a missing information in the commit message and PR description

Thanks, updated, from tablets

Main goal is to make `tokenAwareHostPolicy.Pick` code less convoluted.
But there are little functional changes:
Before this PR hosts from tablets are not being shuffled
There are three host information sources: tablets, meta and tokenring,
Before this PR if tables returned no hosts for given token, in certain cases code did not consider meta or tokenring.
And same for meta, it did not consider tokenring.
After this PR, it starts goes through tablets, meta and tokenring, if current host sources yield no hosts it proceed to next one.
@dkropachev dkropachev force-pushed the dk/simlify-tokenaware-policy-pick branch from ce30161 to b79ba6d Compare June 25, 2024 20:23
@sylwiaszunejko sylwiaszunejko merged commit 74675d1 into scylladb:master Jun 26, 2024
1 check passed
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