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

IndexedDbVFS rollback does not work unless cache_size=0 #33

Closed
rhashimoto opened this issue Jan 19, 2022 · 1 comment
Closed

IndexedDbVFS rollback does not work unless cache_size=0 #33

rhashimoto opened this issue Jan 19, 2022 · 1 comment
Labels
bug

Comments

@rhashimoto
Copy link
Owner

@rhashimoto rhashimoto commented Jan 19, 2022

In the demo, the following SQL inserts two rows into a table, then inserts another row within a transaction that is rolled back:

PRAGMA journal_mode=delete;
PRAGMA cache_size=1024;
DROP TABLE IF EXISTS tbl;
CREATE TABLE IF NOT EXISTS tbl (x PRIMARY KEY, y);
REPLACE INTO tbl VALUES ('foo', 6), ('bar', 7);

BEGIN;
INSERT INTO tbl VALUES ('not', 'stored');
ROLLBACK;
SELECT * FROM tbl;

There should be only two rows in the table, but instead there are three. If cache_size is set to 0 then it works correctly.

@rhashimoto rhashimoto added the bug label Jan 19, 2022
@rhashimoto
Copy link
Owner Author

@rhashimoto rhashimoto commented Jan 19, 2022

The problem is in the new "no journal" journal, which is a hack to take advantage of the fact that we can always store all database changes from SQLite transaction as a single atomic write to IndexedDB. The VFS skips passing all the journaled blocks back and just cancels the write.

This is working fine for the database as stored in IndexedDB. But it means that SQLite's internal cache of the database isn't rolled back properly. In general it isn't safe for SQLite to keep using that cached data because once the transaction lock is released some other connection could modify the database. But SQLite has a trick where every change to the database increments a counter in the database header. When SQLite gets a new lock, it reads that counter and if it hasn't changed then the cached data should still be valid. Except that now it isn't because the rollback hack allowed the two to diverge.

The solution should be just to increment that counter. We'll see if that works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
Projects
None yet
Development

No branches or pull requests

1 participant