From 08de169f3ff2ccbbb81add4eabd0afdc69dcf2ad Mon Sep 17 00:00:00 2001 From: Remco Westerhoud Date: Tue, 10 Oct 2023 16:46:59 +0200 Subject: [PATCH 1/2] test(engine): verify we don't iterate over deleted keys When iterating in an open transaction, we would create an iterator containing all entries in the database as well as the entries in the transaction cache. This ensures any keys added in the same transaction are also encountered during iterating. We do not do anything with the keys that are deleted within the transaction. As a result, when iterating we will go over keys that are already deleted. This is wrong. The keys are deleted, so we should not encounter them anymore during iterating. (cherry picked from commit cc4511494d483a2789c8919cdbaaac176f88e55f) --- .../db/InMemoryZeebeDbTransactionTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/engine/src/test/java/io/camunda/zeebe/process/test/engine/db/InMemoryZeebeDbTransactionTest.java b/engine/src/test/java/io/camunda/zeebe/process/test/engine/db/InMemoryZeebeDbTransactionTest.java index 95644b077..51a43580e 100644 --- a/engine/src/test/java/io/camunda/zeebe/process/test/engine/db/InMemoryZeebeDbTransactionTest.java +++ b/engine/src/test/java/io/camunda/zeebe/process/test/engine/db/InMemoryZeebeDbTransactionTest.java @@ -8,6 +8,7 @@ package io.camunda.zeebe.process.test.engine.db; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; import io.camunda.zeebe.db.ColumnFamily; import io.camunda.zeebe.db.TransactionContext; @@ -492,6 +493,25 @@ void shouldWriteKeyAfterDeletion() { assertThat(oneColumnFamily.get(oneKey).getValue()).isEqualTo(-2); } + @Test + void shouldNotIterateOverDeletionsInTransaction() throws Exception { + // given + oneKey.wrapLong(1); + oneValue.wrapLong(-1L); + oneColumnFamily.insert(oneKey, oneValue); + transactionContext.getCurrentTransaction().commit(); + + // when - then + transactionContext.runInTransaction( + () -> { + oneColumnFamily.deleteExisting(oneKey); + oneColumnFamily.forEach( + (key, value) -> { + fail("Should not iterate over deleted keys"); + }); + }); + } + private enum ColumnFamilies { DEFAULT, // rocksDB needs a default column family ONE, From 439fd110e774608b8bd3e56224d8ab096b0164e1 Mon Sep 17 00:00:00 2001 From: Remco Westerhoud Date: Tue, 10 Oct 2023 16:47:11 +0200 Subject: [PATCH 2/2] feat(engine): don't iterate over deleted keys When iterating in an open transaction, we would create an iterator containing all entries in the database as well as the entries in the transaction cache. This ensures any keys added in the same transaction are also encountered during iterating. We do not do anything with the keys that are deleted within the transaction. As a result, when iterating we will go over keys that are already deleted. This is wrong. The keys are deleted, so we should not encounter them anymore during iterating. (cherry picked from commit 84be02be63e56025c5c9c43a91364b8433bb0654) --- .../zeebe/process/test/engine/db/InMemoryDbTransaction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/src/main/java/io/camunda/zeebe/process/test/engine/db/InMemoryDbTransaction.java b/engine/src/main/java/io/camunda/zeebe/process/test/engine/db/InMemoryDbTransaction.java index 9180cb36c..3891846fd 100644 --- a/engine/src/main/java/io/camunda/zeebe/process/test/engine/db/InMemoryDbTransaction.java +++ b/engine/src/main/java/io/camunda/zeebe/process/test/engine/db/InMemoryDbTransaction.java @@ -100,6 +100,7 @@ public InMemoryDbIterator newIterator() { final TreeMap snapshot = new TreeMap<>(); snapshot.putAll(database); snapshot.putAll(transactionCache); + deletedKeys.forEach(snapshot::remove); return new InMemoryDbIterator(snapshot); }