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

Avoid locking in tserver when reading online tablets #1100

Merged
merged 2 commits into from Apr 18, 2019

Conversation

@keith-turner
Copy link
Contributor

commented Apr 17, 2019

Profiling Accumulo during running a performance test showed lots of lock
contention for the map of online tablets. Most operations need to read
this map. This change creates a read only snapshot of the online
tablets that is used to avoid lock contention.

keith-turner added some commits Apr 17, 2019

Avoid locking in tserver when reading online tablets
Profiling Accumulo during running a performance test showed lots of lock
contention for the map of online tablets.  Most operations need to read
this map.  This change creates a read only snapshot of the online
tablets that is used to avoid lock contention.

@milleruntime milleruntime added the v2.0.0 label Apr 18, 2019

@milleruntime
Copy link
Contributor

left a comment

This is cool. Did you see performance improvements avoiding all these locks?

@keith-turner

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

This is cool. Did you see performance improvements avoiding all these locks?

A little bit. What I find is that when I get rid of one lock that is causing contention it allows another one to shine. There are still other cases of lock contention that I saw in the profiling data. These only showed up after fixing this.

log.error("Error updating rates for {}", tablet.getExtent(), ex);
}
}
long now = System.currentTimeMillis();

This comment has been minimized.

Copy link
@milleruntime

milleruntime Apr 18, 2019

Contributor

Will dropping the sychronized block in the simple timer cause problems? I am not sure how much of the SimpleTimer is shared and what we should worry about.

This comment has been minimized.

Copy link
@keith-turner

keith-turner Apr 18, 2019

Author Contributor

I don't think it will cause any problems. It was sync before to avoid concurrent mod exceptions. Now it will iterate over the immutable snapshot.

@keith-turner keith-turner merged commit 8b52f3b into apache:master Apr 18, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@keith-turner keith-turner deleted the keith-turner:tablet-concurrent-map branch Apr 18, 2019

@ctubbsii ctubbsii added this to Done in 2.0.0 Jun 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.