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 issue with sparse reading a segment that is being modified. #503

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

Conversation

kjnilsson
Copy link
Contributor

When a sparse read detects a modified segment and re-initialises it is possible that the segment will again be modified before the second read attempt. It is not necessary in this case to check if the segment has been modified as the read plan would have been generated before the segment was re-initialised so this commit introduces a new ra_log_segment:read_sparse_no_checks function that skips the modified check to avoid a crash.

@kjnilsson kjnilsson added this to the 2.16.0 milestone Jan 21, 2025
When a sparse read detects a modified segment and re-initialises
it is possible that the segment will again be modified before the
second read attempt. It is not necessary in this case to check
if the segment has been modified as the read plan would have been
generated before the segment was re-initialised so this commit
introduces a new ra_log_segment:read_sparse_no_checks function
that skips the modified check to avoid a crash.
@kjnilsson kjnilsson force-pushed the read-plan-modified-fix branch from 3c4917b to 87ea602 Compare January 21, 2025 10:47
@@ -319,6 +321,17 @@ read_sparse(#state{index = Index,
read_sparse0(Fd, Indexes, Index, Cache0, Acc, AccFun, 0)
Copy link
Member

Choose a reason for hiding this comment

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

To cut down on duplication a bit read_sparse could be implemented in terms of read_sparse_no_checks, eh?

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