ChainSafe / ChainSafe/lodestar

non-atomic write during the migration of data sidecars

Open
#8,244 2 comments 0 reactions 0 assignees View on GitHub
meta-bug spec-fulu :zebra:
Dominant language
TypeScript
Stars
1.4k
Forks
483
Avg merge
1d 18h
Merged PRs (30d)
156

Description

### Describe the bug

During refactoring of the `dataSidecars` observed that we have a non-atomic write for migration of the data-column sidecars and we reply on just the promise fulfillment to make sure whole operation is finished.

```ts
promises.push(
db.dataColumnSidecarArchive.putManyBinary(
block.slot,
dataColumnSidecars.map(({id, value}) => ({id, value}))
)
);

promises.push(db.dataColumnSidecar.deleteMany(canonicalBlocks.map((block) => block.root)));

await Promise.all(promises);
```

So there the `delete` and `put` are run independant to each other and each can fail individually. This will create inconsistent state on the database. e.g. We may put dataColumns to archive but the delete options failed. Or worst what if the delete works out and put failed and we will lost the data and have to sync again.

### Expected behavior

These both operations should be atomic batch operation. Either it passes fully or fail fully. So we never lost the data or have inconsistent db state.

### Steps to reproduce

The highlighted case is just one case, but there are other places in the code where we can get benifit from fixing this approach.

Can happen with many weird edge cases. Currently it never triggered because `db.close()` tries to complete all requests before closing. But the process can also terminate or even the single promise is yet left to execute in the event queue.

### Additional context

The underlaying `leveldb` have that support for atomic batch requests but our `@lodestar/db` is designed in the way to not utilize that feature.

### Operating system

Linux

### Lodestar version or commit hash

unstable

Contributor guide

Open the contributing guide

Research direction

Start in the @lodestar/db implementation behind dataColumnSidecarArchive.putManyBinary and dataColumnSidecar.deleteMany, then review how db.close() handles pending requests and how the underlying LevelDB batch support is exposed. Done means the migration's archive and delete operations are committed atomically, with failure leaving no inconsistent or lost data.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.