mirror of
https://github.com/aljazceru/turso.git
synced 2025-12-26 04:24:21 +01:00
7045d44fdc
Closes #2421 ## Background We have some kind of transaction-local hack (`start_pages_in_frames`) for bookkeeping how many pages are currently in the in-memory WAL frame cache, I assume for performance reasons or whatever. `wal.rollback()` clears all the frames from `shared.frame_cache` that the rollbacking tx is allowed to clear, and then truncates `shared.pages_in_frames` to however much its local `start_pages_in_frames` value was. ## Problem In `complete_append_frame`, we check if `frame_cache` has that key (page) already, and if not, we add it to `pages_in_frames`. However, `wal.rollback()` never _removes_ the key (page) if its value is empty, so we can end up in a scenario where the `frame_cache` key for `page P` exists but has no frames, and so `page P` does not get added to `pages_in_frames` in `complete_append_frame`. This leads to a checkpoint data loss scenario: - transaction rolls back, has start_pages_in_frames=0, so truncates shared pages_in_frames to an empty vec. let's say `page P` key in `frame_cache` still remains but it has no frames. - The next time someone commits a frame for `page P`, it does NOT get added to `pages_in_frames` because `frame_cache` has that key (although the value vector is empty) - At some point, a checkpoint checkpoints `n` frames, but since `pages_in_frames` does not have `page P`, it doesn't actually checkpoint it and all the "checkpointed" frames are simply thrown away - very similar to the scenario in #2366 ## Fix Remove the `start_pages_in_frames` hack entirely and just make `pages_in_frames` effectively the same as `frame_cache.keys`. I think we could also just get rid of `pages_in_frames` and just use `frame_cache.contains_key(p)` but maybe Pere can chime in here Reviewed-by: Pere Diaz Bou <pere-altea@homail.com> Closes #2422