Merge all bench into gigasim - #4390
Conversation
PR SummaryLow Risk Overview Monitoring and repo hygiene follow that consolidation: Prometheus scrape job Gigasim logging setup changes so Per the broader PR scope (not all visible in the diff snippet), gigasim also picks up the former sims’ workloads and knobs—e.g. Reviewed by Cursor Bugbot for commit c07c5fb. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
There was a problem hiding this comment.
Reworks gigasim's workload and tooling: an ERC20-or-native TransactionType with holder-keyed token balance slots, ledger payload packing so blocks can carry more than 2,000 transactions, a MaxTps throttle, logs moved under DataDir, configurable FlatKV cache sizes, an opt-in pprof server, and storage_gc_ metrics on the garbage collector. Nothing in the changed code breaks correctness: the removed symbols have no callers left, and the claim/clean logic now keeps the open log file. The one scout claim does not hold, and no Go toolchain was available, so nothing was built or tested.
Non-blocking
1 finding on the changed lines, as inline comments.
- The title says "Merge all bench into gigasim", but
sei-db/bench/blocksimandsei-db/bench/cryptosimare untouched, and the description is still the unfilled template. A reviewer can't tell whether the default changes (10,000 tx × 200 B blocks,FlushIntervalBlocks1, queue depth 20,LookbackWindow100,000, checkpoints every 5,000 blocks) are deliberate retuning or leftovers from a local experiment. - Scout codex's one finding was dropped: it said
emptyLogDir(gigasim.go:811) falls off the end without a return. The merged tree ends the function withreturn nil; the diff hunk just stops before that line.
1 nit, not posted on the code
sei-db/common/metrics/pprof.go:26— The README's examplePprofAddrof":6060"binds every interface, which exposes/debug/pprof/cmdlineand heap and goroutine dumps to anyone who can reach the host. Alocalhost:6060example would be a safer thing to copy.
seidroid review · decision approve · session 57e08a802c044d3a9c021a2f547c8af2 · turn resp_claude_d235748bcc71485a54b8dce3b6109b6b · item 04d99a2e828150f4a49310f817b270e3
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4390 +/- ##
==========================================
+ Coverage 55.53% 55.55% +0.01%
==========================================
Files 2176 2178 +2
Lines 169020 169086 +66
==========================================
+ Hits 93868 93935 +67
+ Misses 75147 75146 -1
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| senderTopic := indexedAddressTopic(addressFromKey(txn.srcAccount)) | ||
| receiverTopic := indexedAddressTopic(addressFromKey(txn.dstAccount)) | ||
|
|
||
| gasUsed := receiptGasUsedBase + rand.Int64Range(0, receiptGasUsedSpan) |
There was a problem hiding this comment.
ERC20 receipt gas and reported gas differ
Each ERC20 receipt stores 52,000 plus a random value in [0, 18,000). The block header and gigasim_gas_used_total use Erc20GasPerTransaction, which defaults to 50,000. A sum of receipt gas and the gigagas panel tell different stories. The README table says the ERC20 gas is 50,000.
Fix: Write Erc20GasPerTransaction into the ERC20 receipt, and make CumulativeGasUsed the sum of those values. Keep the random draw only if the README says that receipt gas is a separate synthetic range.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 602499e. Configure here.
|
@seidroid review |
There was a problem hiding this comment.
Since my last review the PR deletes blocksim, cryptosim and walsim along with their dashboards, moves cryptosim's setup script and Prometheus/AWS instructions into gigasim, and adds per-shard atomic hit/miss counting to the view cache's metrics; the gigasim workload changes are the same as before. Nothing in the new code blocks: no references to the deleted benchmarks remain, and I'm resolving my resume-compatibility thread because the description now says old data directories must be rebuilt. Codex's missing-return claim on emptyLogDir is dropped again because the function ends in return nil in the merged tree. No Go toolchain was available, so nothing was built or tested.
Non-blocking
- View cache hit and miss counters now reach OTel only once per
MetricsScrapeInterval, not on every read. Dashboards will see those rates move in steps of one scrape interval, and a short interval is the only way to get finer resolution.
1 nit, not posted on the code
sei-db/common/metrics/pprof.go:26— The README's examplePprofAddrof":6060"binds every interface. That exposes/debug/pprof/cmdlineand heap and goroutine dumps to anyone who can reach the host, which matters most on the AWS setup the README now describes. Alocalhost:6060example would be safer to copy.
seidroid review · decision approve · session 57e08a802c044d3a9c021a2f547c8af2 · turn resp_claude_30065f9b53eb0a149559e10707a65adb · item 4f35470bda2d5684b7ed57c830c8ec1a
Findings: 0 blocking | 1 non-blocking | 0 posted inline

Makes gigasim the single storage benchmark. It already ran the whole
GigaStorageManagerstack end to end; this closes the gaps that kept blocksim (block ledger only), cryptosim (state DB only) and walsim (WAL only) around, then deletes those three and their dashboards.Workload
TransactionType:erc20(default) ortransfer.Erc20InteractionsPerAccounttokens and sends one of them.ledgerPayloadpacks several per entry, still within the 2 MiB byte budget.Config changes
TransactionType(defaulterc20)Erc20GasPerTransaction(50,000)MaxTps(0 = unthrottled), replacingMaxBlocksPerSecondPprofAddr(empty = off),MutexProfileFraction(0),BlockProfileRate(0)AccountCacheSizeBytes(1 GiB),CodeCacheSizeBytes(1 GiB),StorageCacheSizeBytes(4 GiB)MaxBlocksPerSecond(useMaxTps)LogDir: logs always go toDataDir/logsTransactionsPerBlock2,000 → 10,000BytesPerTransaction256 → 200MaxPendingExecutionQueueSize100 → 20FlushIntervalBlocks10 → 1Erc20ContractSize4,096 → 2,048LookbackWindow1,000,000 → 100,000CheckpointIntervalSeconds60 → 300CheckpointBlockInterval0 → 5,000Erc20InteractionsPerAccount: number of tokens each account holdsHotErc20ContractProbability: share of holdings (and so of transfers) in the hot token setTransactionsPerBlock: now capped at 1,000,000 instead of 2,000standard.jsonrenamed tofull-node.json, withLookbackWindow1,000,000archive-node.json:LookbackWindow-1, keeps all historyvalidator.json:LookbackWindow0debug.json:MaxBlocksPerSecond10 →MaxTps1,000TransactionTypeexplicitly, and none setLogDirMetrics and dashboard
gigasim_gas_used_total,gigasim_store_keys_written_total{store}, and atypelabel ongigasim_transactions_executed_total.cryptosimrenamed togigasim.Removed
sei-db/bench/blocksim,cryptosimandwalsim, plus theblocksimandcryptosimdashboards. Cryptosim's Ubuntu setup script moves togigasim/tools/, and its Prometheus/Grafana and AWS instructions move into the gigasim README.Risk
sei-db/controller) now publishesstorage_gc_*metrics (cut lines, per-store rollback floors, per-store prune duration and errors). Pruning decisions are unchanged, but nodes running the collector will export these metrics too.MaxBlocksPerSecondorLogDirnow fail to load (unknown fields are rejected).Validation
-race.