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

Fix recycled WAL detection when wal_compression is enabled #12643

Closed
wants to merge 5 commits into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented May 11, 2024

I think the point of the if (end_of_buffer_offset_ - buffer_.size() == 0) was to only set recycled_ when the first record was read. However, the condition was false when reading the first record when the WAL began with a kSetCompressionType record because we had already dropped the kSetCompressionType record from buffer_. To fix this, I used first_record_read_ instead.

Also, it was pretty confusing to treat the WAL as non-recycled when a recyclable record first appeared in a non-first record. I changed it to return an error if that happens.

@facebook-github-bot
Copy link
Contributor

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ajkr ajkr force-pushed the fix-recycled-wal-detection branch from 86b9bcc to a5ac7b9 Compare May 15, 2024 19:21
@facebook-github-bot
Copy link
Contributor

@ajkr has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@ajkr has updated the pull request. You must reimport the pull request before landing.

@ajkr ajkr requested a review from hx235 May 15, 2024 19:59
@facebook-github-bot
Copy link
Contributor

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

db/log_test.cc Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@ajkr merged this pull request in c72ee45.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants