From 0be76e0b55de9deb7346919eb4c761ae7f4cc4c6 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Thu, 11 Jun 2026 14:28:32 +0200 Subject: [PATCH] statedb: avoid revision bumps for missing deletes Currently, (*writeTxnState).delete increments the table revision before checking whether the target object even exists. A committed delete of a missing object could thus advance the table revision without producing a corresponding revision index entry or watch notification. Fix this by moving the revision allocation until after the primary key lookup succeeded and any CompareAndDelete guard passed. Signed-off-by: Tobias Klauser --- db_test.go | 21 +++++++++++++++++++++ write_txn.go | 9 ++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/db_test.go b/db_test.go index 1c9b205..b818803 100644 --- a/db_test.go +++ b/db_test.go @@ -835,6 +835,27 @@ func TestDB_Revision(t *testing.T) { require.Equal(t, writeRevision, startRevision+1, "revision incremented on Insert") readRevision = table.Revision(db.ReadTxn()) require.Equal(t, writeRevision, readRevision, "committed transaction changed revision") + + // Committed no-op deletes do not increment the revision. + txn = db.WriteTxn(table) + _, hadOld, err := table.Delete(txn, &testObject{ID: 2}) + require.NoError(t, err) + require.False(t, hadOld) + writeRevision = table.Revision(txn) + txn.Commit() + require.Equal(t, readRevision, writeRevision, "missing Delete did not increment revision") + readRevision = table.Revision(db.ReadTxn()) + require.Equal(t, readRevision, readRevision, "committed missing Delete did not change revision") + + // Committed deletes of existing objects increment the revision. + txn = db.WriteTxn(table) + _, hadOld, err = table.Delete(txn, &testObject{ID: 1}) + require.NoError(t, err) + require.True(t, hadOld) + writeRevision = table.Revision(txn) + txn.Commit() + require.Equal(t, readRevision+1, writeRevision, "revision incremented on Delete") + require.Equal(t, writeRevision, table.Revision(db.ReadTxn()), "committed found Delete changed revision") } func TestDB_GetList(t *testing.T) { diff --git a/write_txn.go b/write_txn.go index cd87d9c..9730bcc 100644 --- a/write_txn.go +++ b/write_txn.go @@ -235,15 +235,12 @@ func (txn *writeTxnState) delete(meta TableMeta, guardRevision Revision, data an return object{}, false, ErrTransactionClosed } - // Look up table and allocate a new revision. + // Look up table. tableName := meta.Name() table := txn.tableEntries[meta.tablePos()] if !table.locked { return object{}, false, tableError(tableName, ErrTableNotLockedForWriting) } - oldRevision := table.revision - table.revision++ - revision := table.revision // Delete from the primary index first to grab the object. // We assume that "data" has only enough defined fields to @@ -260,11 +257,13 @@ func (txn *writeTxnState) delete(meta TableMeta, guardRevision Revision, data an if guardRevision > 0 { if obj.revision != guardRevision { idIndex.insert(idKey, obj) - table.revision = oldRevision return obj, true, ErrRevisionNotEqual } } + table.revision++ + revision := table.revision + // Remove the object from the revision index. binary.BigEndian.PutUint64(txn.revKey[:], obj.revision) txn.mustIndexWriteTxn(meta, RevisionIndexPos).delete(txn.revKey[:])