Skip to content

Conversation

and-cb
Copy link
Collaborator

@and-cb and-cb commented Jul 22, 2025

The orphan page list has some complicated logic around it so that it is "crash recoverable" to some extent. This logic is showing some flaws that are described in #151 and are reproduced by the new test in this PR.

Rather than trying to fix the logic, this PR changes it completely: pushing new items to the orphan page list now always appends items to the end of it. At commit time, if there's enough room, the orphans are moved to the start of the list, overwriting the reclaimed orphans.

This makes commits more expensive, but guarantees that all orphans are eventually used (as long as snapshots are non-decreasing), and also the amortized cost of adding orphans is still O(1).

TODO

  • Update comments and documentation
  • Maybe add a check to push() to ensure that snapshots are non-decreasing
  • Rename the test push_pop_2 to something better
  • Remove the debug code in pop()

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Jul 22, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@and-cb and-cb force-pushed the andrea/overwriting-orphan-list branch from 7f616f1 to a162dc9 Compare July 23, 2025 21:34
Copy link
Collaborator

@BrianBland BrianBland left a comment

Choose a reason for hiding this comment

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

:shipit:

@and-cb and-cb force-pushed the andrea/overwriting-orphan-list branch from a162dc9 to 5be9c4c Compare August 18, 2025 22:57
@and-cb and-cb marked this pull request as ready for review August 18, 2025 22:58
@and-cb and-cb merged commit f31e31b into main Aug 19, 2025
18 checks passed
@and-cb and-cb deleted the andrea/overwriting-orphan-list branch August 19, 2025 04:00
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.

3 participants