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

c-deps: backport RocksDB range deletion performance fix #26877

Closed
wants to merge 1 commit into from

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jun 21, 2018

The current implementation of range deletion tombstones in RocksDB
suffers from a performance bug that causes excessive CPU usage on every
read operation in a database with many range tombstones. Dropping a
large table can easily result in several thousand range deletion
tombstones in one store, resulting in an unusable cluster as documented
in #24029.

Backport a refactoring of range deletion tombstone that fixes the
performance problem. This refactoring has also been proposed upstream as
facebook/rocksdb#4014.

A more minimal change was also proposed in facebook/rocksdb#3992--and
that patch better highlights the exact nature of the bug than the patch
backported here, for those looking to understand the problem. But this
refactoring, though more invasive, gets us one step closer to solving a
related problem where range deletions can cause excessively large
compactions (#26693). These large compactions do not appear to brick the
cluster but undoubtedly have some impact on performance.

Fix #24029.

Release note: None

@benesch benesch requested review from bdarnell, tbg, petermattis and a team June 21, 2018 05:41
@cockroach-teamcity
Copy link
Member

This change is Reviewable

The current implementation of range deletion tombstones in RocksDB
suffers from a performance bug that causes excessive CPU usage on every
read operation in a database with many range tombstones. Dropping a
large table can easily result in several thousand range deletion
tombstones in one store, resulting in an unusable cluster as documented
in cockroachdb#24029.

Backport a refactoring of range deletion tombstone that fixes the
performance problem. This refactoring has also been proposed upstream as
facebook/rocksdb#4014.

A more minimal change was also proposed in facebook/rocksdb#3992--and
that patch better highlights the exact nature of the bug than the patch
backported here, for those looking to understand the problem. But this
refactoring, though more invasive, gets us one step closer to solving a
related problem where range deletions can cause excessively large
compactions (cockroachdb#26693). These large compactions do not appear to brick the
cluster but undoubtedly have some impact on performance.

Fix cockroachdb#24029.

Release note: None
@benesch
Copy link
Contributor Author

benesch commented Jun 21, 2018

CI passed so... :shipit:?

@petermattis
Copy link
Collaborator

I'm taking a look.


Review status: :shipit: complete! 0 of 0 LGTMs obtained


Comments from Reviewable

@petermattis
Copy link
Collaborator

I left some comments on the upstream PR.


Review status: :shipit: complete! 0 of 0 LGTMs obtained


Comments from Reviewable

@benesch
Copy link
Contributor Author

benesch commented Jul 16, 2018

This was taken care of by #27520. h/t @petermattis.

@benesch benesch closed this Jul 16, 2018
@benesch benesch deleted the rdperf branch July 16, 2018 13:46
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.

storage: dropping a large table will brick a cluster due to compactions
3 participants