From b503d7ff9ac251601c17e31b8a9d84512f4d4858 Mon Sep 17 00:00:00 2001 From: Elliot Chance Date: Thu, 23 Feb 2023 20:47:09 -0500 Subject: [PATCH] sequences: Fixing concurrent "NEXT VALUE FOR" (#143) This fixes a bug where concurrent transactions using "NEXT VALUE FOR" would make the sequence invisible to the other concurrent transaction. The properties of a sequence (such as the `INCREMENT BY`, etc) are held in the same record as the next value. Since the next value of a sequence needs to be atomic (and separate from the transaction isolation) a `ROLLBACK` on a transaction that contains an `ALTER SEQUENCE` will not undo any changes. Ideally, the properties of a `SEQUENCE` can be stored in a separate location on disk. However, for now it's a documented limitation. --- docs/alter-sequence.rst | 10 +++ docs/file-format.rst | 22 +++++++ tests/next-value-for.sql | 128 +++++++++++++++++++++++++++++++++++++++ vsql/storage.v | 19 +++++- 4 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 tests/next-value-for.sql diff --git a/docs/alter-sequence.rst b/docs/alter-sequence.rst index 5a1e539..b2c98f1 100644 --- a/docs/alter-sequence.rst +++ b/docs/alter-sequence.rst @@ -44,6 +44,16 @@ Examples -- msg: ALTER SEQUENCE 1 -- COL1: 1 COL2: 2 +Caveats +------- + +The properties of a sequence (such as the ``INCREMENT BY``, etc) are held in the +same record as the next value. Since the next value of a sequence needs to be +atomic (and separate from the transaction isolation) a ``ROLLBACK`` on a +transaction that contains an ``ALTER SEQUENCE`` will not undo any changes. + +This was noted in :doc:`file-format` under *Notes for Future Improvements*. + See Also -------- diff --git a/docs/file-format.rst b/docs/file-format.rst index 9900aea..2acb65c 100644 --- a/docs/file-format.rst +++ b/docs/file-format.rst @@ -207,6 +207,14 @@ A Schema Object (has the ``S`` prefix) contains a schema definition. The serialization does not need to be explained in detail here. You can check the code for ``Schema.bytes()`` and ``new_schema_from_bytes()`` respectively. +Sequence Object +--------------- + +A Sequence Object (has the ``Q`` prefix) contains the definition and next value +for a sequence. Since a sequence's next value needs to be atomic, this creates +some special rules that only apply to updating or incrementing a sequence. See +*Notes for Future Improvements* below or *Caveats* in :doc:`alter-sequence`. + Table Object ------------ @@ -214,6 +222,20 @@ A Table Object (has the ``T`` prefix) contains a table definition. The serialization does not need to be explained in detail here. You can check the code for ``Table.bytes()`` and ``new_table_from_bytes()`` respectively. +Notes for Future Improvements +----------------------------- + +Sequences +^^^^^^^^^ + +The properties of a sequence (such as the ``INCREMENT BY``, etc) are held in the +same record as the next value. Since the next value of a sequence needs to be +atomic (and separate from the transaction isolation) a ``ROLLBACK`` on a +transaction that contains an ``ALTER SEQUENCE`` will not undo any changes. + +Ideally, the properties of a ``SEQUENCE`` can be stored in a separate location +on disk. + Notes ----- diff --git a/tests/next-value-for.sql b/tests/next-value-for.sql new file mode 100644 index 0000000..f07fcf9 --- /dev/null +++ b/tests/next-value-for.sql @@ -0,0 +1,128 @@ +CREATE SEQUENCE seq1; +CREATE TABLE bar (baz INTEGER); +INSERT INTO bar (baz) VALUES (NEXT VALUE FOR seq1); +INSERT INTO bar (baz) VALUES (NEXT VALUE FOR seq1); +SELECT * FROM bar; +UPDATE bar SET baz = NEXT VALUE FOR seq1; +SELECT * FROM bar; +-- msg: CREATE SEQUENCE 1 +-- msg: CREATE TABLE 1 +-- msg: INSERT 1 +-- msg: INSERT 1 +-- BAZ: 1 +-- BAZ: 2 +-- msg: UPDATE 2 +-- BAZ: 3 +-- BAZ: 4 + +/* connection 1 */ +CREATE SEQUENCE seq1; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 2 */ +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 3 */ +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +-- 1: msg: CREATE SEQUENCE 1 +-- 1: COL1: 1 COL2: 2 +-- 2: COL1: 3 COL2: 4 +-- 3: COL1: 5 COL2: 6 + +/* connection 1 */ +CREATE SEQUENCE seq1; +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +COMMIT; +/* connection 2 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +COMMIT; +/* connection 3 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +COMMIT; +-- 1: msg: CREATE SEQUENCE 1 +-- 1: msg: START TRANSACTION +-- 1: COL1: 1 COL2: 2 +-- 1: msg: COMMIT +-- 2: msg: START TRANSACTION +-- 2: COL1: 3 COL2: 4 +-- 2: msg: COMMIT +-- 3: msg: START TRANSACTION +-- 3: COL1: 5 COL2: 6 +-- 3: msg: COMMIT + +/* connection 1 */ +CREATE SEQUENCE seq1; +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +ROLLBACK; +/* connection 2 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +ROLLBACK; +/* connection 3 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +ROLLBACK; +-- 1: msg: CREATE SEQUENCE 1 +-- 1: msg: START TRANSACTION +-- 1: COL1: 1 COL2: 2 +-- 1: msg: ROLLBACK +-- 2: msg: START TRANSACTION +-- 2: COL1: 3 COL2: 4 +-- 2: msg: ROLLBACK +-- 3: msg: START TRANSACTION +-- 3: COL1: 5 COL2: 6 +-- 3: msg: ROLLBACK + +/* connection 1 */ +CREATE SEQUENCE seq1; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +ROLLBACK; +/* connection 2 */ +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +START TRANSACTION; +/* connection 1 */ +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 2 */ +COMMIT; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +-- 1: msg: CREATE SEQUENCE 1 +-- 1: COL1: 1 COL2: 2 +-- 1: msg: START TRANSACTION +-- 1: COL1: 3 COL2: 4 +-- 1: msg: ROLLBACK +-- 2: COL1: 5 COL2: 6 +-- 2: msg: START TRANSACTION +-- 1: COL1: 7 COL2: 8 +-- 2: msg: COMMIT +-- 2: COL1: 9 COL2: 10 + +/* connection 1 */ +CREATE SEQUENCE seq1; +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 2 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 3 */ +START TRANSACTION; +VALUES NEXT VALUE FOR seq1, NEXT VALUE FOR seq1; +/* connection 1 */ +COMMIT; +/* connection 2 */ +COMMIT; +/* connection 3 */ +COMMIT; +-- 1: msg: CREATE SEQUENCE 1 +-- 1: msg: START TRANSACTION +-- 1: COL1: 1 COL2: 2 +-- 2: msg: START TRANSACTION +-- 2: COL1: 3 COL2: 4 +-- 3: msg: START TRANSACTION +-- 3: COL1: 5 COL2: 6 +-- 1: msg: COMMIT +-- 2: msg: COMMIT +-- 3: msg: COMMIT diff --git a/vsql/storage.v b/vsql/storage.v index cc7005b..0611b4f 100644 --- a/vsql/storage.v +++ b/vsql/storage.v @@ -210,8 +210,13 @@ fn (mut f Storage) sequence_next_value(name Identifier) !i64 { next_sequence := sequence.next()! key := 'Q${canonical_name}'.bytes() + + // Important: All other objects have to use the current transaction ID when + // modifying an entity so that the change is not visible to other transaction. + // However, a sequence's number needs to be atomic and modifys the value in + // place. old_obj := new_page_object(key, sequence.tid, 0, sequence.bytes()) - new_obj := new_page_object(key, f.transaction_id, 0, next_sequence.bytes()) + new_obj := new_page_object(key, sequence.tid, 0, next_sequence.bytes()) for page_number in f.btree.update(old_obj, new_obj, f.transaction_id)! { f.transaction_pages[page_number] = true } @@ -232,8 +237,18 @@ fn (mut f Storage) update_sequence(old_sequence Sequence, new_sequence Sequence) } key := 'Q${canonical_name}'.bytes() + + // Important: All other objects have to use the current transaction ID when + // modifying an entity so that the change is not visible to other transaction. + // However, a sequence's number needs to be atomic and modifys the value in + // place. + // + // Unfortunately, this also means that we can't guarantee the properties + // (INCREMENET BY, etc) AND the next value (which needs to be atomic outside + // of any transaction). So a ROLLBACK on a sequence will not undo such + // property modifications. old_obj := new_page_object(key, old_sequence.tid, 0, old_sequence.bytes()) - new_obj := new_page_object(key, f.transaction_id, 0, new_sequence.bytes()) + new_obj := new_page_object(key, old_sequence.tid, 0, new_sequence.bytes()) for page_number in f.btree.update(old_obj, new_obj, f.transaction_id)! { f.transaction_pages[page_number] = true }