Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Index: removing an already removed entry #560

Open
MarcusDenker opened this issue Jan 9, 2024 · 3 comments
Open

Index: removing an already removed entry #560

MarcusDenker opened this issue Jan 9, 2024 · 3 comments

Comments

@MarcusDenker
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

1 participant