vlcn-io / cr-sqlite

Convergent, Replicated SQLite. Multi-writer and CRDT support for SQLite
https://vlcn.io
MIT License
2.61k stars 69 forks source link

Duplicate `seq` for the same `db_version` #380

Open jeromegn opened 9 months ago

jeromegn commented 9 months ago

There's at least one scenario where duplicate seq values may exist for the same db_version.

Reproduction:

sqlite> create table foo (a INTEGER PRIMARY KEY NOT NULL, b INT);
sqlite> select crsql_as_crr('foo');
sqlite> .mode column
sqlite> insert into foo values (1, 1);
sqlite> insert into foo values (2, 2);
sqlite> insert into foo values (3, 3);
sqlite> select crsql_db_version();
crsql_db_version()
------------------
3
sqlite> begin;
sqlite> insert into crsql_changes values ("foo", X'010904', "b", 4, 1, 1, X'9B1D5BD20C7A47C28FC482B33DCFC412', 1, 0);
sqlite> insert into crsql_changes values ("foo", X'010905', "b", 5, 1, 2, X'9B1D5BD20C7A47C28FC482B33DCFC412', 1, 0);
sqlite> select crsql_next_db_version();
crsql_next_db_version()
-----------------------
4
sqlite> commit;
sqlite> select "table", hex(pk), cid, val, col_version, db_version, hex(coalesce(site_id, crsql_site_id())), cl, seq from crsql_changes;
table  hex(pk)  cid  val  col_version  db_version  hex(coalesce(site_id, crsql_site_id()))  cl  seq
-----  -------  ---  ---  -----------  ----------  ---------------------------------------  --  ---
foo    010901   b    1    1            1           1864106555524AA0B5FA926536A7C047         1   0
foo    010902   b    2    1            2           1864106555524AA0B5FA926536A7C047         1   0
foo    010903   b    3    1            3           1864106555524AA0B5FA926536A7C047         1   0
foo    010904   b    4    1            4           9B1D5BD20C7A47C28FC482B33DCFC412         1   0
foo    010905   b    5    1            4           9B1D5BD20C7A47C28FC482B33DCFC412         1   0
tantaman commented 9 months ago

We decided that the appropriate fix here is to increment db_version each time we see a new db_version in the current transaction rather than only incrementing if the seen version is greater than the current version.

Todo:

We currently only bump versions iff:

Separately, we should be assigning to x + 1 rather than just x if x is greater than the currently assigned next_db_vesrion