Write synchronously in LevelDBSnapshot.Commit and prevent data loss.

[jsolman]

I have been puzzled for a while on an issue where syncing MainNet continued to throw exceptions when I sync the chain using machines with high performance cpu and lower disk performance. I discovered the batch write in commit was not being performed synchronously, which seems to lead to the LevelDB current snapshot for the block being created with missing data, which potentially leads to data loss upon persisting the next block. Usually this surfaces as some exception when trying to persist the block.

Update: I believe the root cause now also includes the Blockchain running Import messages on different OS threads.

This fixes part of the problem to fix to the root cause issue I reported way back here as what I thought was a neo-vm issue at the time: neo-project/neo-vm/issues/53

[jsolman]

I’m not entirely sure this has eliminated the issue I have been seeing yet. Testing is still ongoing.

[jsolman]

I think Blockchain may also need to use a PinnedDispatcher: https://getakka.net/articles/actors/dispatchers.html#pinneddispatcher

The updated ImportBlocks plugin issues imports in separate messages, and the Blockchain needs to execute them on the same OS thread for safety.

[jsolman]

Actually, I believe using the PinnedDispatcher for the Blockchain is more important than synchronizing the batched writes. We probably don’t have to synchronize the batch writes after all.

[erikzhang]

Doing so will significantly reduce performance and is not necessary.

[jsolman]

I agree, I will close this and re-open another one with the PinnedDispatcher for the Blockchain. That will fix the issue I think.

[erikzhang]

What is the reason for using PinnedDispatcher?

[jsolman]

It seems if the Blockchain gets a second import message and runs on a different thread it may be possible for the blockchain snapshot that is obtained to not be the latest snapshot of the LevelDB database. This is what my tests are indicating. If the same OS thread is used such as if calling ImportBlocks once with an Iterable that will get all blocks in one call as the existing Plugin does, no problem is encountered during syncing.

[jsolman]

The Blockchain.currentSnapshot will be right because it uses interlocked exchange from the same thread, but the new one obtained during OnPersist could be wrong possibly, because it obtains it again from the DB possibly on a different OS thread.

[jsolman]

The actor’s state itself should be guaranteed to have memory visibility if the thread changes, but things outside the actor like in LevelDB may not have consistent memory visibility if the thread has changed.