Add transaction to rel deletions

[acquamarin]

1. This PR adds the transaction logic for rel-deletion.
2. Refactors the prepare-commiting logic for lists:
   Instead of commiting each list at a time, we commit each relTable at a time.

[semihsalihoglu-uw]

Why do you check emptyListInPersistentStore? Whether the list was empty in persistent store seems irrelevant to whether v has updates or not.

[acquamarin]

The purpose of the hasUpdates() is to check whether we need to update the list/header/metadata for the current nodeOffset.
We want hasUpdates to return true if we want to update the list/header/metadata for a nodeOffset. When we insert a new node, we set the emptyListIPersistentStore to true in the listUpdates for that new node. During commiting, we need to fix the header for that newly inserted node. As a result, we need hasUpdates to return true in order to fix the header.

[semihsalihoglu-uw]

It looks like you want the else if as your first case because it is possible that there was a large list L, which became empty because all of its rels were deleted and we want to still call appendToLargeList on L. Then the current if can be the new else if.

Also this is a hard piece of code to read so comment on each case what the case is handling the way you comment on the current else if that it is the large list optimization etc.

[acquamarin]

I think we already have a comment for the large list optimization.
The if (listUpdates.emptyListInPersistentStore) is a very special branch which is designed for newly inserted nodes.

[semihsalihoglu-uw]

As a second comment I'm now debating if this first if clause is justified. Why don't you just have 2 cases:

1. if the list is a large list and there are only updates;
2. else

So you don't differentiate between whether the persistent list was empty or not and just call iterator->updateList(...), which will check whether to fill from the persistent store or not. I'm assuming updateList() should actually just work even if the persistent store is empty. It would simplify the code.

Given that you do this, I have a follow up question: Aside from here do we ever use emptyListInPersistentStore? If we remove this if case, it may be unused and if so, we should remove it too.

[acquamarin]

As we discussed, let's keep the listsUpdates.emptyListInPersistentStore branch here since the logic is quite different from the newly inserted nodes and other nodes:

If the node is newly inserted, we should only read from the updateStore.
If the node is not newly inserted, we should both read from the persistent and update store.

btw: renamed the emptyListInPersistentStore -> newlyAddedNode

[semihsalihoglu-uw]

The previous tests above, e.g., deleteRelsFromSmall/LargList or deleteRelsFromUpdateStore also delete multiple rels? What exactly is this testing? If this test doesn't add much, let's remove it.

[acquamarin]

1. deleteRelsFromSmallList is to test the behaviour of deleting multiple rels from a small list. Specifically, i want to test whether the CSR structure has been fixed properly after each deletion.
2. deleteRelsFromLargelist is to test the behaviour of deleting multiple rels from a large list. Specifically, i want to test whether the largelist has been fixed properly after each deletion.
3. deleteRelsFromUpdateStore is quite different from deleting from persistent store. When we want to delete rels from updateStore, we can simply remove the rels from the insertedRelTupleIdxes of updateStore.
   BTW: i am not deleting the old tests, i am just extending them to test transaction behaviour

[semihsalihoglu-uw]

Is this deleting all rels or almost all rels? Maybe remove this test and add two tests that test the edge cases of deleting all rels from a small and large list.

[acquamarin]

This test is to test deleting large number of rels from a large list.
Addded two tests for deleting all rels from small/large list.