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

Spine drop batches once logical and physical compaction frontiers are empty #399

Closed
wants to merge 3 commits into from

Conversation

antiguru
Copy link
Member

@antiguru antiguru commented Jun 28, 2023

Once the physical logical compaction frontier is empty, there are no times that can be represented in a a trace, which means that we're good to drop all contents.

This fixes #398.

@antiguru antiguru marked this pull request as ready for review June 28, 2023 11:47
@antiguru antiguru requested a review from frankmcsherry June 28, 2023 11:47
@frankmcsherry
Copy link
Member

I don't think this is right. Physical compaction can be allowed up through [] and it just means that you are only allowed to ask for a cursor through [], not that the trace is invalidated. Logical compaction would be the thing that when it reaches [] would mean that the trace is not obliged to be correct for any time (as none are in advance of / beyond []).

@antiguru antiguru changed the title Spine drop batches once physical compaction frontier is empty Spine drop batches once logical compaction frontier is empty Jun 28, 2023
Comment on lines 485 to 490
// If the logical frontier is empty, we are not obliged to be able to represent any time,
// since no time is in advance of `[]`. In the case, we can drop all contents.
if self.logical_frontier.is_empty() {
self.drop_batches();
}

Copy link
Member

Choose a reason for hiding this comment

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

Is there an explainer for why the code is here, rather than in e.g. set_logical_compaction?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the code to set_logical_compaction and insert. As far as I can tell these are the only places where logical compaction is advanced, and new batches are inserted.

antiguru added 3 commits July 19, 2023 10:16
Once the logical compaction frontier is empty, there are no times
that can be represented in a a trace, which means that we're good to
drop all contents.

Signed-off-by: Moritz Hoffmann <[email protected]>
@antiguru antiguru changed the title Spine drop batches once logical compaction frontier is empty Spine drop batches once logical and physical compaction frontiers are empty Jul 24, 2023
@antiguru
Copy link
Member Author

Closing because this approach is not correct.

@antiguru antiguru closed this Dec 20, 2023
@antiguru antiguru deleted the fix_398 branch April 4, 2024 17:28
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.

Holding on to a trace with physical/logical compaction to the empty frontier stalls compaction
2 participants