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

Add read to file:open/2 options in ra_lib:sync_file/1 #452

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Jul 2, 2024

This fixes an issue where checkpoints which were promoted to snapshots would become empty. We need to file:open/2 the checkpoint file with the read option as well as write since write without read truncates the file.

We also add another server restart to the checkpoint promotion case which catches the failure - the servers failed to restart since the promoted snapshot is empty.

the-mikedavis and others added 2 commits July 2, 2024 12:13
From the [file:open/2] docs:

> `write` - The file is opened for writing. It is created if it does not
> exist. If the file exists and `write` is not combined with `read`, the
> file is truncated.

Without this change, checkpoints which are promoted to snapshots end up
as empty snapshot files.

[file:open/2]: https://www.erlang.org/doc/apps/kernel/file.html#open/2

Co-authored-by: Karl Nilsson <[email protected]>
This ensures that we can recover from a snapshot which was promoted from
a checkpoint. The restart earlier in the test case ensures that we can
recover from a checkpoint but that doesn't provide an insights about
checkpoint promotion.

Without the fix for `ra_lib:sync_file/1` in the parent commit, this case
fails on the new calls to `ra:restart_server/2`.
@kjnilsson kjnilsson merged commit 2becc7b into main Jul 2, 2024
10 checks passed
@the-mikedavis the-mikedavis deleted the md/ra_lib-sync_file-open-with-read branch July 2, 2024 19:31
@the-mikedavis the-mikedavis added this to the 2.12.1 milestone Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants