erigontech / erigontech/erigon

Lock-free parallel execution: derive per-block changesets post-hoc, remove changeset accumulator from the exec path

Open
#21,106 1 comment 0 reactions 1 assignee Claimed by @mh0lt View on GitHub
performance tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## Background

PR #21088 added a `changesetMu` mutex on `SharedDomains` (and `FlushPendingUpdatesLocked` / `ComputeCommitmentLocked` variants) as a band-aid to serialize the parallel commitment calculator's swap-and-record window against the apply goroutine's `DomainPut`/`DomainDel`. That closed the off-by-one wrong-trie-root cluster, `TestRecreateAndRewind`, and all ~227 race-detector hits across the `EXEC3_PARALLEL=true` race-test matrix groups — but at the cost of serializing apply-side writes during compute.

## The problem

The "current changeset accumulator" is unwind-side machinery: a sidecar that records per-block prev-value diffs so a later unwind can reconstruct the pre-block state. Execution should be forward-only and not be concerned with it. Today the parallel calculator swaps a *global* accumulator pointer to route block N's branch writes into block N's saved CS, and the apply loop writes through that same pointer — hence the need for the band-aid lock.

## Proposed direction

Derive per-block changesets **post-hoc from sd entries** (now tx-granular) at `sd.Flush` time, instead of maintaining the accumulator during execution. Then:

- delete `changesetMu` and the `Lock/UnlockChangesetAccumulator` + `*Locked` API surface
- delete the swap dance in `committer.go computeWithBlockAccumulator`
- delete the `SetChangesetAccumulator` / `GetChangesetAccumulator` / `SavePastChangesetAccumulator` API
- delete the `domain == kv.CommitmentDomain` exemptions in `SharedDomains.DomainPut` / `DomainDel`

### Smallest first step (Option A0)

`SharedDomainsCommitmentContext.deferCommitmentUpdates` already exists and is enabled for parallel-applying-blocks (`exec3.go:217`). Branches accumulate in `pendingUpdate` and `FlushPendingUpdates` replays them. The remaining inline-write paths to chase are `encodeAndStoreCommitmentState`'s `[state]` marker write and the `concurrentTrieContextFactory` ETL drain — route those through the deferred mechanism too, then the lock window collapses to a single single-threaded flush.

### Review note

@AskAlexSharov suggested folding `changesetMu` into the existing `latestStateLock` in #21088 review — kept separate to avoid widening the high-traffic `latestStateLock` (held on every `Put`/`Del`/`GetLatest`/`GetAsOf`) to cover the calculator's whole `ComputeCommitment` window. The lock-free refactor here makes that question moot.

## Acceptance

- `changesetMu` and the associated swap API removed
- all four parallel race-test matrix groups still report 0 race-detector hits
- all Bucket C tests (`TestBlockchainHeaderchainReorgConsistency`, `TestLongerForkHeaders/Blocks`, `TestCallTraceUnwind`, `TestTxLookupUnwind`, `TestLowDiffLongChain`, `TestRecreateAndRewind`) still pass under `EXEC3_PARALLEL=true`
- benchmark: apply-side throughput during compute no longer serialized

Related: #21088, #21017

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.