Skip to content

cmd/utils/app: process all files passed to import command#21513

Open
yperbasis wants to merge 4 commits into
mainfrom
yperbasis/import-all-block-files
Open

cmd/utils/app: process all files passed to import command#21513
yperbasis wants to merge 4 commits into
mainfrom
yperbasis/import-all-block-files

Conversation

@yperbasis
Copy link
Copy Markdown
Member

@yperbasis yperbasis commented May 29, 2026

Problem

The Hive legacy-cancun BlockchainTests suite intermittently fails 4 tests with client did not start: timed out waiting for container startup:

  • walletReorganizeOwners_{Cancun,Istanbul,London,Paris}

(e.g. this run). They took ~181s, just over Hive's 180s container-startup timeout. The same test on other forks (Shanghai/Berlin) and ForkStressTest_* sit at 146–150s — right at the cliff.

Root cause

The import command documents multi-file support:

USAGE: erigon import [command options] <filename> (<filename 2> ... <filename N>)

but importChain only processed cliCtx.Args().First(), silently ignoring the rest. That forced Hive's erigon entrypoint into a one-process-per-block-file loop. For walletReorganizeOwners (235 block files) that is 235 full erigon startups — measured at ~0.5s/process × 261 = 145s of pure startup, while actual block execution is ~40ms each. go-ethereum has no such problem: its entrypoint imports every block in one geth import invocation.

Fix

Iterate every file argument in a single process, tolerating per-file failures when several files are given (matching go-ethereum). This lets the hive entrypoint import all blocks in one invocation, collapsing hundreds of process startups into one (~150s → ~10s).

It also force-disables the embedded MCP server for the one-shot import (--mcp.disable, alongside the existing NAT / downloader / external-consensus disables) — a batch import has no use for it.

Companion change (required)

The hive entrypoint must pass all block files in one invocation: ethereum/hive#1519. Order matters — the hive change depends on this one (on an old erigon, import /blocks/* would import only the first block).

Testing

  • New unit tests (import_cmd_test.go): all-files iteration, per-file failure tolerance, single-file error surfacing.
  • Real-binary check: erigon import a.rlp b.rlp now attempts both files in one process (previously stopped after the first).
  • make lint clean; make erigon integration builds.

The import command documented multi-file support but only processed
cliCtx.Args().First(), silently ignoring the rest. This forced Hive's
erigon entrypoint to restart a full erigon process per block file; on
block-heavy BlockchainTests (walletReorganizeOwners, ForkStressTest) the
~0.5s/process startup overhead times hundreds of blocks blew past Hive's
180s container-startup timeout.

Iterate every file argument in a single process, tolerating per-file
failures when several files are given, matching go-ethereum. This lets the
hive entrypoint import all blocks in one invocation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the erigon import command to process every positional file argument in a single initialized process, aligning implementation with the command’s documented multi-file behavior and reducing repeated startup overhead for Hive block imports.

Changes:

  • Replaces single-file import handling with ordered iteration over all provided files.
  • Adds helper behavior to continue processing remaining files after per-file failures.
  • Adds unit tests covering multi-file processing and error surfacing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cmd/utils/app/import_cmd.go Adds multi-file import iteration via importFiles.
cmd/utils/app/import_cmd_test.go Adds tests for processing order, continued iteration after failures, and single-file error propagation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The import command builds the full node via eth.New, which starts the
embedded MCP SSE server by default (mcp.addr/mcp.port). A one-shot block
import has no use for it, so force-disable it via --mcp.disable alongside
the existing NAT/downloader/external-consensus disables.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread cmd/utils/app/import_cmd.go
yperbasis and others added 2 commits May 29, 2026 16:57
A SIGINT/SIGTERM was treated like a per-file import failure, so on a multi-file import the interrupt only aborted the current file and the loop continued with the next. Return a sentinel errInterrupted from ImportChain and stop the importFiles loop on it, so user cancellation aborts the whole command.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants