Skip to content

reduce seal lock contention - #4314

Merged
cody-littley merged 2 commits into
mainfrom
cjl/seal-lock-contention
Sep 24, 2026
Merged

cody-littley merged 2 commits into
mainfrom
cjl/seal-lock-contention

Conversation

@cody-littley

Copy link
Copy Markdown
Contributor

At around 160k TPS, lock contention during the commit_seal_store phase prevents us from increasing system velocity. This PR should significantly reduce that contention.

@cursor

cursor Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes concurrency and error timing on the hot commit path; correctness still depends on version checks and bricking on partial seal failure.

Overview
Reduces lock contention during commit/seal by overlapping work that used to run serially.

viewManager.Commit now seals all shards in parallel via miscPool (sealShardsLocked), instead of calling shard.Commit one after another while holding versionLock. The manager-level pre-pass that checked every shard for out-of-service before sealing is removed; each shard.Commit now refuses to seal under its own lock if the shard is out of service, avoiding a race where a shard could go bad between check and seal. commitLocked is folded into Commit; failure handling still bricks the manager and waits for all seal tasks to finish.

FlatKV block commit seals the four backing view stores (account, code, storage, misc) concurrently on the same miscPool, with errors joined after Wait.

Reviewed by Cursor Bugbot for commit ba7c9e2. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.74576% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.68%. Comparing base (fb6db35) to head (ba7c9e2).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
sei-db/db_engine/view/view_manager_impl.go 78.12% 7 Missing ⚠️
sei-db/db_engine/view/shard.go 33.33% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4314      +/-   ##
==========================================
- Coverage   67.46%   66.68%   -0.78%     
==========================================
  Files        2179     2081      -98     
  Lines      167361   158382    -8979     
==========================================
- Hits       112903   105617    -7286     
+ Misses      54448    52755    -1693     
  Partials       10       10              
Flag Coverage Δ
sei-chain-pr 89.27% <74.28%> (?)
sei-db 74.50% <ø> (ø)
sei-db-state-db ?
sei-db-state-db-pr 83.75% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/state_db/sc/flatkv/store_write.go 81.57% <100.00%> (+3.04%) ⬆️
sei-db/db_engine/view/shard.go 85.79% <33.33%> (-0.32%) ⬇️
sei-db/db_engine/view/view_manager_impl.go 90.89% <78.12%> (+0.18%) ⬆️

... and 99 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedSep 24, 2026, 1:38 PM

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parallelising the per-shard seal and the per-store commit is sound: lock ordering is preserved (no shard-lock holder reaches back for versionLock, and every pending.await happens after the shard lock is released), errs[i]/per-store result variables are written to disjoint slots and read after wg.Wait(), and expectedVersion is read after c.currentVersion++ so the sanity check still compares like for like. No blockers; the notes below are about a now-stale pool contract and two structural points.

Findings: 0 blocking | 3 non-blocking | 3 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 3 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-db/db_engine/view/view_manager_impl.go
Comment thread sei-db/db_engine/view/view_manager_impl.go
Comment thread sei-db/state_db/sc/flatkv/store_write.go
Comment thread sei-db/db_engine/view/view_manager_impl.go Outdated
c.brickLocked(err)
return nil, 0, err
}
c.miscPool.Submit(func() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sealShardsLocked submits one shard.Commit per shard and wg.Waits, all under versionLock. A misc-pool worker that hits a fold failure calls reportFoldFailure → brick(), which blocks on that same versionLock. If every worker is parked there, the seal tasks never start, Wait never returns, and the lock is never released.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVM, that seems safe for now since miscPool is an elasticPool

@cody-littley
cody-littley added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit f978be0 Sep 24, 2026
68 checks passed
@cody-littley
cody-littley deleted the cjl/seal-lock-contention branch September 24, 2026 14:07
yzang2019 added a commit that referenced this pull request Sep 25, 2026
* main: (21 commits)
  Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345)
  Remove the oracle module behind a v6.8 upgrade (#4319)
  fix(seidb): report only the current migration boundary on the snapshot gauge (#4327)
  fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322)
  Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330)
  Add giga.storage.receipts to toggle the Autobahn receipt store (#4333)
  optimize gather phase (#4326)
  Regenerate the Unreleased changelog as a plain PR list (#4336)
  Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332)
  Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318)
  Add dashboard and topology option for Autobahn e2e (#4167)
  Add Giga fetch/serve and BlockDB prune metrics (#4329)
  Add eth_getLogs to the EVM-only Giga RPC (#4308)
  Generate v6.8 precompiles (#4320)
  Add [giga] app.toml section and honor it on the Autobahn node (#4323)
  feat(evmonly): add eth_estimateGas via existing libraries (#4325)
  Use Pebble batch directly in SS (#4300)
  Fix pruning issue in SS causing huge disk spike (#4321)
  Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316)
  reduce seal lock contention (#4314)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants