[indexer]: Handle race conditions with new object-based dependency checks

[tomxey]

Previously dependency check was checking if global order of previous tx is already present in the global order table AND that it's status is NOT Started (so we checked if indexing for this tx is already fully finished)

Currently we are checking if transaction input objects are present in the DB (in other words, we check if objects created by parent transactions are already indexed), but that doesn't mean that parent transactions are indexed fully.
They still may have e.g. global order not assigned or they may still want to remove some objects or they may want to update the display table, etc.

this creates a race condition that may lead to failures like:

---- write_api::test_parallel_shared_object_updates stdout ----

thread 'write_api::test_parallel_shared_object_updates' (78126163) panicked at crates/iota-indexer/tests/rpc-tests/write_api.rs:656:25:
tx: TransactionDigest(CtCFeKVDVEgMqUseunL3qAA538Z8ZR2MVVGxqQfXPHka) should have bigger order than it's deps: {TransactionDigest(C1f4eFtw2R2octZa7y4o35Vb2UnL6R1gGQwCdfMHzEDW)}

Part of the issue could be solved by making sure global order is assigned in checkpoint indexing before changes to objects table are written.

To be investigated if this is enough to solve all the issues.

[kodemartin]

The ordering side-effect is not essential as we don't plan to provide support for ordered responses with mixed optimistic and checkpointed transactions.

The problem lies with the removal of objects that is done concurrently with the creation.

So currently, if a transaction TxA creates an Object (id_i, version_i) that also wraps an object (id_j, version_j) and a dependent transaction TxB depends on (id_i, version_i) among other things to unwrap (id_j, version_j) into (id_j, version_j+1) , then there is not guarantee that the wrapping will happen before the unwrapping.

To guarantee that we should ensure that deletions happen only if the version to be deleted is larger than the existing version on the objects table.

[tomxey]

Apart from the case described above that can be solved by checking object version before deletion, there is also a similar case with object creation.

Scenario (this already assumes we have applied the fix to check object version before deletion):

Legend:

• object B_j - 🍌
• object B_j+1 - 🍌✨
• object A_i, the wrapper - 📦

TxA(reads 🍌, deletes 🍌, creates 📦) - wrap 🍌 inside of 📦
TxB(reads 📦, deletes 📦, creates 🍌✨) - unwrap 🍌✨ from 📦

Steps:

• user executes TxA via indexer, it gets executed on the node, passes dependency check (🍌 exists in indexer DB)
• indexer hangs before actual indexing, in the meantime user executes followup TxB via node directly
• TxA and TxB arrive via checkpoint path, object changes are being indexed, tx_global_order is not yet assigned (possible because those are independent tasks), effectively in the DB object 📦 is deleted, object 🍌✨ exists (this is a correct state)
• optimistic indexing of TxA unhangs, assigns global order (not yet taken by checkpoint path), indexes object changes and commits, DB state after this: object 📦 exists, object 🍌✨ exists
• checkpoint indexing finishes, global orders are assigned, incorrect objects state remains, transaction TxB that should remove 📦 is already marked as indexed so indexer will not automatically recover from this

such ghost object can only be created on optimistic path,
deletion of object on optimistic path followed by creation of ghost object on checkpoint path is impossible, since for object to be deleted on optimistic path, it's creation needs to be already indexed, and this will prevent checkpoint indexing pipeline from indexing the same change again

Remedies

There is currently nothing preventing optimistic pipeline from writing an object that was already deleted at later version,
simple version check will not work here since there is no entry in the DB to compare the version with at the moment of insertion.

Writing to tx_global_order table at the beginning of checkpoint indexing before object tables are written would solve this, since it would effectively disable optimistic indexing for txs that are about to be checkpoint indexed.
This could however negatively impact the ingestion speed, since it would add a new sequential step to the pipeline. (we could however parallelize everything else, so only persist of tx_global_order and objects would need to be sequential, pipeline would be slowed down only if tx_global_order + objects commit would be the slowest component of the parallel commit)

To be investigated what is the optimal solution for this.

[tomxey]

such ghost object can only be created on optimistic path,
deletion of object on optimistic path followed by creation of ghost object on checkpoint path is impossible, since for object to be deleted on optimistic path, it's creation needs to be already indexed, and this will prevent checkpoint indexing pipeline from indexing the same change again

Actually there is a possibility of indexing the same object change on checkpoint path more than once.

If indexer would create ObjectA in scope of TX1 but then crash right away without accounting that given checkpoint is indexed, then it is possible that followup optimistic transaction TX2 will be indexed after that that depends on ObjectA and that deletes ObjectA.

Checkpoint indexing then, when resumed will restart from the checkpoint that was last processed and write ObjectA again in scope of TX1, but later when processing the TX2 that is supposed to delete this object, it will not do it because TX2 will already be marked as optimistically indexed and checkpoint indexing is skipping such changes. So we will be left out with a ghost ObjectA.

I find it way less likely to happen than the previous case, since it requires indexer to crash and those are not normal operations. That's a theoretical possibility however.

The solutions pointed out above (version check on delete, ordering object updates after tx_global_order updates) will not solve this.
Ideally we could check on insertion whether such object already existed or was deleted at a higher version, but it's not a trivial check, since we don't have this information directly available in the objects table.

My suggestion would be to move this to a separate issue.