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

[WIP/DNM] table: fix behavior on new table block error #915

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

asubiotto
Copy link
Member

Table block creation can fail for a variety of reasons, but we were only
handling the happy case. This commit only overwrites the active block if the
new table block was successfully created, and logs a noop record to the WAL on
error to not block subsequent wal records logs.

Bug found with DST.

This happens in cases where a txn must be logged although the operation failed.
In this case, we do not want side-effects of entries, only to increment the
next expected txn.
Table block creation can fail for a variety of reasons, but we were only
handling the happy case. This commit only overwrites the active block if the
new table block was successfully created, and logs a noop record to the WAL on
error to not block subsequent wal records logs.
Copy link
Member

@metalmatze metalmatze left a comment

Choose a reason for hiding this comment

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

Once more very interesting. 👍

@asubiotto asubiotto changed the title table: fix behavior on new table block error [WIP/DNM] table: fix behavior on new table block error Jun 26, 2024
Not doing so could lead to a sequence of events that could result in dropped
data:

- x is written to active block b1
- b1 is rotated, b2 becomes the active block before this has been written to
the WAL.
- A snapshot occurs of b2, the snapshot contains no data.
- A crash happens, on restart, the snapshot loads b2, an empty block.

If NewTableBlock was synchronously written in the above case, b1 would not be
overwritten by b2 until b2's creation has been synchronously written to the
WAL which can subsequently be observed in recovery. There could still be an
issue if the snapshot truncates the WAL, but that is an issue for another
commit.
@asubiotto asubiotto mentioned this pull request Jun 26, 2024
2 tasks
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.

None yet

2 participants