Respect updates to st_table during replay#3937
Merged
Conversation
when seeing a unique constraint violation after replay
Prior to this commit, we had special handling for deletes from `st_table` during replay, but we did not pair them with inserts to form updates, instead only treating the delete as a dropped table. With this commit, we record inserts to `st_table` which will form update pairs during replay, and handle them appropriately, updating the table's schema and not dropping the table. There is a tricky case where a table exists but is empty, and then within a single transaction: - The table undergoes a migration s.t. its `table_access` or `primary_key` changes. - At least one row is inserted into the table. In this case, the in-memory table structure will not exist at the point of the `st_table` insert, but will be created before the corresponding `st_table` delete, meaning there will be two conflicting `st_table` rows resident. To handle this case, `CommittedState` tracks a side table, `replay_table_updated`, which stores a `RowPointer` to the correct most-recent `st_table` row for the migrating table. I've also renamed the one previously-extant replay-only side table, `table_dropped`, to include the `replay_` prefix, which IMO improves clarity. And I've made it so `replay_table_dropped` is cleared at the end of each transaction, as the previous behavior of continuing to ignore a table that should be unreachable masked errors which would have been helpful when debugging this issue.
Printing the full table contents was helpful while debugging, but is too much to see in logs normally.
Good catch, Tyler!
jdetter
pushed a commit
that referenced
this pull request
Dec 30, 2025
# Description of Changes Prior to this commit, we had special handling for deletes from `st_table` during replay, but we did not pair them with inserts to form updates, instead only treating the delete as a dropped table. With this commit, we record inserts to `st_table` which will form update pairs during replay, and handle them appropriately, updating the table's schema and not dropping the table. There is a tricky case where a table exists but is empty, and then within a single transaction: - The table undergoes a migration s.t. its `table_access` or `primary_key` changes. - At least one row is inserted into the table. In this case, the in-memory table structure will not exist at the point of the `st_table` insert, but will be created before the corresponding `st_table` delete, meaning there will be two conflicting `st_table` rows resident. To handle this case, `CommittedState` tracks a side table, `replay_table_updated`, which stores a `RowPointer` to the correct most-recent `st_table` row for the migrating table. I've also renamed the one previously-extant replay-only side table, `table_dropped`, to include the `replay_` prefix, which IMO improves clarity. And I've made it so `replay_table_dropped` is cleared at the end of each transaction, as the previous behavior of continuing to ignore a table that should be unreachable masked errors which would have been helpful when debugging this issue. This PR also includes an extended error message when encountering a unique constraint violation while replaying, which I found helpful while debugging. # API and ABI breaking changes N/a # Expected complexity level and risk 2 - replay is complicated and scary, but this PR isn't gonna make things *more* broken than they already were. # Testing - [x] Manually replayed a commitlog which included a migration that altered a table's `table_access`, which was previously broken but now replays successfully.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
Prior to this commit, we had special handling for deletes from
st_tableduring replay, but we did not pair them with inserts to form updates, instead only treating the delete as a dropped table.With this commit, we record inserts to
st_tablewhich will form update pairs during replay, and handle them appropriately, updating the table's schema and not dropping the table.There is a tricky case where a table exists but is empty, and then within a single transaction:
table_accessorprimary_keychanges.In this case, the in-memory table structure will not exist at the point of the
st_tableinsert, but will be created before the correspondingst_tabledelete, meaning there will be two conflictingst_tablerows resident. To handle this case,CommittedStatetracks a side table,replay_table_updated, which stores aRowPointerto the correct most-recentst_tablerow for the migrating table.I've also renamed the one previously-extant replay-only side table,
table_dropped, to include thereplay_prefix, which IMO improves clarity. And I've made it soreplay_table_droppedis cleared at the end of each transaction, as the previous behavior of continuing to ignore a table that should be unreachable masked errors which would have been helpful when debugging this issue.This PR also includes an extended error message when encountering a unique constraint violation while replaying, which I found helpful while debugging.
API and ABI breaking changes
N/a
Expected complexity level and risk
2 - replay is complicated and scary, but this PR isn't gonna make things more broken than they already were.
Testing
table_access, which was previously broken but now replays successfully.