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

storage: allow compactor to delete sstable files in compacted ranges #24137

Closed

Conversation

spencerkimball
Copy link
Member

This is a test. DO NOT MERGE.

@spencerkimball spencerkimball requested review from benesch, tbg and a team March 22, 2018 00:03
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@spencerkimball
Copy link
Member Author

No warranties on this. I haven't even tried it out on a real example. It's just recycled code from the original PR, refurbished for the current state of the compactor. I'm unclear on the source of the slowdown, so it's not obvious to me that this will address the root problem.

// If the key spans don't overlap, then check whether they're
// "nearly" contiguous.
if aggr.EndKey.Compare(sc.StartKey) < 0 {
// First, if voided, the spans must be exactly contiguous or we
// are unable to aggregate.
if aggr.Voided {
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess this is where we'll end up not being able to determine contiguous ranges for a single delete files in range for a much larger key span, unfortunately.

FYI, the original code in the PR to efficiently drop tables sent the full key span of the table being dropped as part of the suggested compaction so this gap could be bridged with some extra logic in the compactor.

Copy link
Member

Choose a reason for hiding this comment

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

No, this wouldn't be enough. Sending the span along only proves that the key space was supposed to be empty at some past instant in time. Replicas could have reappeared into that empty keyspace and you wouldn't be allowed to delete them. I think what we'd need really is a tighter coupling with the store. Whenever the compactor finds a gap it wants to bridge, it asks the store how far it can go (and takes the min with how far it wants to go). The store adds placeholders for any handed-out keyspan until the compactor acks them back.

@spencerkimball spencerkimball deleted the drop-sstables branch October 22, 2018 01:11
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.

None yet

3 participants