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

Dead Page Reference Count Bug Fix #181

Merged
merged 33 commits into from
Feb 27, 2025
Merged

Dead Page Reference Count Bug Fix #181

merged 33 commits into from
Feb 27, 2025

Conversation

bhaskarbora2
Copy link
Collaborator

@bhaskarbora2 bhaskarbora2 commented Dec 18, 2024

This MR is to release the fix for ref count bug in page recycler's dead page recycling flow. This issue crops up when volume trimmer and page recycler retries a dead page recycling task for a page after recovery. The solution is to track largest_slot_offset in recycler which will be checked at the time of receiving any new dead_page recycling request from volume trimmer. Any request having lower or equal slot_offset compare to largest-slot-offset will be ignored.

The MR also adds in a histogram plot to show where all we are hitting the bug over the entire seed range. This is done through an existing page ref count test.

#13

@bhaskarbora2 bhaskarbora2 self-assigned this Dec 18, 2024
@bhaskarbora2 bhaskarbora2 changed the title Page Reference Count Bug Fix Dead Page Reference Count Bug Fix Dec 18, 2024
Copy link
Collaborator

@tonyastolfi tonyastolfi left a comment

Choose a reason for hiding this comment

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

General comment: on functions that return Status or StatusOr, usually it's best to declare as noexcept. My advice would be to do this on functions whose signature we modify, or newly added functions; as opposed to just going through and changing a bunch of existing signatures.

Copy link
Collaborator

@tonyastolfi tonyastolfi left a comment

Choose a reason for hiding this comment

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

Here's what I have so far... I need to review the page_recycler.cpp changes in more detail. Will post another review later.

@tonyastolfi tonyastolfi merged commit 8ca38fa into main Feb 27, 2025
4 checks passed
@tonyastolfi tonyastolfi deleted the ref_count_bug_fix branch February 27, 2025 15:34
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.

2 participants