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

Remove locks for tablets flag on Conn #196

Merged

Conversation

dkropachev
Copy link
Collaborator

@dkropachev dkropachev commented Jun 20, 2024

Removes unnecessary Conn.mu locking on accessing tabletsRoutingV1 flag.
Conn.mu is overused, less it is used less contention is yielded along with more performance.

@dkropachev dkropachev force-pushed the dk/remove-tablet-routing-v1-lock branch from 45e3c09 to 6eb181d Compare June 20, 2024 20:27
@roydahan roydahan requested a review from sylwiaszunejko June 23, 2024 23:57
@roydahan
Copy link
Collaborator

@dkropachev can you please add the description also to the commit message?

@dkropachev
Copy link
Collaborator Author

@dkropachev can you please add the description also to the commit message?

Done

@dkropachev dkropachev force-pushed the dk/remove-tablet-routing-v1-lock branch from 6eb181d to 358800f Compare June 24, 2024 11:36
policies.go Outdated
@@ -651,7 +651,6 @@ func (t *tokenAwareHostPolicy) Pick(qry ExecutableQuery) NextHost {
var replicas []*HostInfo

if qry.GetSession() != nil && qry.GetSession().tabletsRoutingV1 {
t.tablets.mu.Lock()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why this is here, the commit message is about removing Conn.mu locking

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was wierd merge issue, it is gone after rebase

@sylwiaszunejko
Copy link
Collaborator

pls rebase on master

@dkropachev dkropachev force-pushed the dk/remove-tablet-routing-v1-lock branch from 358800f to a08be15 Compare June 27, 2024 14:12
Removes unnecessary `Conn.mu` locking on accessing `tabletsRoutingV1` flag.
`Conn.mu` is overused, less it is used less contention is yielded along with more performance.
@dkropachev dkropachev force-pushed the dk/remove-tablet-routing-v1-lock branch from a08be15 to 7dba59c Compare June 27, 2024 14:14
@dkropachev
Copy link
Collaborator Author

pls rebase on master

done

@sylwiaszunejko sylwiaszunejko merged commit 258cf8e into scylladb:master Jun 28, 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