Soil icon indicating copy to clipboard operation
Soil copied to clipboard

Index: removing an already removed entry

Open MarcusDenker opened this issue 1 year ago • 3 comments

This is an edge case for the multiple concurrent transaction case as seen in testDoWithTransAction

-> we open tx1 -> we open tx2 -> in tx2 we remove key 1 -> we commit tx2

After this, tx1 should still see key 1, as it was only removed in tx2. #testDoWithTransAction is test for exactly that case and it works.

But now let's remove the key 1 in tx1, too, that is, we do this at the end of #testDoWithTransAction

"We should be able to remove it here, too"

        tx1 root removeKey: 1.
	counter := 0.
	tx1 root do: [ :each |
		self assert: (each beginsWith: 'bar').
		counter := counter + 1].
	self assert: counter equals: 1.

Expected

-> we should be able to remove, as long as we do not commit. A commit should I think then not be possible, as the other transaction was already commited.

What happens -> removeKey tags value as removed again (restoring the value to set removeValue correctly) -> This looks the same as before -> as this case is not distinguishable from the remove in the other transaction, do: will now restore just the same -> assert fails as counter is 2

MarcusDenker avatar Jan 09 '24 14:01 MarcusDenker

The same problem means that changes are ingored, too.

Nothing should prevent us tx1 to change the value (locally).

"We should be able to change it locally here"
	tx1 root at: 1 put: #bar3.
	self assert: (tx1 root at: 1) equals: #bar3.

But the tests fails

MarcusDenker avatar Jan 09 '24 14:01 MarcusDenker

We have already a skipped test that fails due to the same reason I think:

SoilIndexedDictionaryTest>>#testRemoveKeyWithTwoTransactions

           "but removeKey: does not see it, we can remove it without error"
           tx root removeKey: #two ifAbsent: [ self fail ].

 	 self assert: tx root size equals: 1.   "fails here"

	"but commiting it will fail, as we have commited the remove in t2"
	self should: [tx commit] raise: SoilObjectHasConcurrentChange

The remove is not working as the value is restored when checking for #size

MarcusDenker avatar Feb 08 '24 14:02 MarcusDenker

a halt in SoilIndexIterator>>#historicValueAt:ifAbsent: is the best place to check what happens

-> when a value is removed, it has ID 0:0 -> we try to restore it

Now if we remove a value and commit, but then remove it again, the situation is exactly the same as if we would not remove the second time: the value is removed.

So when we ask e.g. for size, it sees the removed value and as we do not have any way to know that it has been removed in this transaction, too, we restore just the same as if the second remove would not have happend. We need to not restore if a values is removed in the current transaction.

Possible soltutions:

  • encode (part of) the transaction ID in the removed
  • We want to manage removed/added values on the page level, maybe this allows other solutions

MarcusDenker avatar Feb 12 '24 16:02 MarcusDenker