[Pebble Storage] Refactor indexing new blocks

[zhangchiqing]

Addressing review comments

This PR makes it concurrent safe to index new blocks.

Running the concurrency test 100 times and passed:

go test --failfast --tags=relic  -run=TestIndexConcurrent -count=100
PASS
ok      github.com/onflow/flow-go/storage/pebble/procedure      6.850s

[jordanschalm]

I don't think the approach here provides any concurrency safety. We need to hold the lock until the write batch is committed.

To address these cases in general, I think we need to define and acquire the lock at the level where the write batch is instantiated and committed. For example, one lock protecting access to the entire block insertion procedure.

[jordanschalm]

Suppose we have 2 threads A and B concurrently writing blocks with a common parent (that initially has no children), the sequence diagram would look like:

sequenceDiagram
    par
        A ->> DB: starts write batch
    and
        B ->> DB: starts write batch
    end
    A -> DB: acquire lock
    activate A
    A -> DB: read Children (nil)
    A -> DB: write Chidren [c1]
    A -> DB: release lock
    deactivate A
    B -> DB: acquire lock
    activate B
    B -> DB: read Children (nil)
    B -> DB: write Chidren [c2]
    B -> DB: release lock
    deactivate B
    par
        A ->> DB: commit write batch
    and
        B ->> DB: commit write batch
    end

Loading

We should end up with children [c1,c2], but instead we will end up with either [c1] or [c2].

The lock added here does not provide any guarantee that this function will operate correctly when called in parallel, because the lock is released before the write is committed.

Suppose we have some database operation P consisting of a set of reads R and a set of writes W, where the writes are dependent on the values read in R. In general, to safely synchronize P, I think we need to:

• acquire Lock_P, the lock protecting access to P
• perform all reads in R, and memorize the values
• instantiate the write batch
• perform all writes in W
• commit the write batch
• release Lock_P

[zhangchiqing]

Nice diagram. And thanks for the feedback.

It's addressed here

I used the callback to release the lock after the batch update is committed.

Also added a test case to reproduce and verify the fix.

[zhangchiqing]

Consolidate and merge all changes to leo/v0.33-pebble-storage-to-review. The changes in this PR can still be a reference.