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

Continue to compact batches that can use it #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frankmcsherry
Copy link
Member

This PR extends the cases in which a trace may introduce phantom batches for the purposes of spurring compaction. Previously, this would not happen if there was just one non-empty batch. However, this was defective if that one batch had times that would collapse if only logical compaction were to be applied.

It is hard to know whether logical compaction will help generally without doing the work, but we can introduce a cheap surrogate which is that as long as the since bound for the batch is not beyond the upper bound for the batch, then there is still some potential for compaction if since were to advance.

We don't know that anything good will happen in general, but for totally ordered times this is a sufficient test (once since is beyond upper, no further logical compaction will occur).

@frankmcsherry
Copy link
Member Author

This PR has a bit of a smell in that since only indicates for what times the updates are guaranteed correct, it does not guarantee that updates were compacted up through this time. For example, if we merged two batches and did not apply logical compaction, we would need to take the upper bounds of their since frontiers, even though the resulting batch is not guaranteed to be compacted that far.

I'm not sure this is actively harmful, but it might result in missed opportunities.

@frankmcsherry
Copy link
Member Author

Talking out loud: this PR also has the potential defect that if you are holding back the compaction frontier, then we don't expect that since will catch up to upper, as we will continually merge together

real batch: { lower1, upper1 }
fake batch: { lower2, upper2 }

where upper1 = lower2, and (unfortunately) the compaction frontier will not be beyond upper2.

Pondering ...

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.

1 participant