Make ExtraIndexer checkpoints reorg-safe by a-shannon · Pull Request #2465 · ergoplatform/ergo

a-shannon · 2026-08-12T12:58:25Z

Summary

Persist the exact full-chain header represented by each ExtraIndexer checkpoint and rebuild legacy, malformed, or interrupted checkpoints under schema 7.
Write forward index rows and checkpoint metadata in one LevelDB batch, bind cached transaction sections to their selected header, and serialize extra-cache misses with writes.
Retry transient catch-up gaps, reconcile restart/reorg ordering against the valid full-block chain, and fail closed when a multi-step rollback cannot be persisted safely.

Why

#2442 made the running indexer track an exact header, but restart still reconstructed that identity from the current best header at the stored height. During a fork, the persisted rows could represent branch A while the reloaded state claimed branch B. Replacement block events could then be applied over the wrong rows before the delayed rollback notification.

This change persists provenance instead of inferring it, validates checkpoint shape and terminal mappings on startup, and drives recovery from the current valid full-block chain. Missing block sections and transient header/full-block disagreement are deferred and retried rather than being recorded as caught up.

Validation

ExtraIndexerSpecification: 23/23
HistoryStorageSpec: 4/4
LDBKVStoreSpec: 3/3

The regressions cover restart onto a competing branch, replacement events before rollback, buffered catch-up followed by reorg, missing transaction sections, malformed/interrupted checkpoints, rollback write failure, cache write failures and retries, an in-flight cache-miss/write race, and production actor startup through the event stream.

Operational notes

Existing non-empty schema-6 extra indexes rebuild once under schema 7.
Forward rows and checkpoint metadata are committed atomically. Rollback remains multi-batch: a durable rollback marker is written first, and a later write failure stops only the ExtraIndexer actor. The node keeps running, and startup validation rebuilds the optional index on the next node restart.
This affects the optional derived extra index only. It does not change consensus validation, UTXO state, P2P serialization, or API schemas.

jozanek

Verified this fixes the catch-up-stall finding from #2293/#2416 — root cause removed (header selection now follows the best full chain), plus generation-guarded retries and self-driven reconciliation, with the stall scenarios regression-tested. Findings: 2 MINOR, 3 NIT inline; no protocol/consensus concerns.

jozanek · 2026-08-12T15:56:49Z


  protected def historyStorage: HistoryStorage = _history.historyStorage

+  protected def fullChainHeaderAtHeight(height: Int): Option[Header] = {


MINOR fullChainHeaderAtHeight re-derives chainStatusKey + BestChainMarker inline even though FullBlockProcessor.isInBestFullChain already encapsulates this check, and the ErgoHistory startup block similarly hand-rolls the validity-key hash that HeadersProcessor.validityKey derives. That's three copies of history's storage layout that can silently drift apart if the encoding ever changes. Suggestion: expose static key-derivation/accessor helpers (e.g. lift isInBestFullChain so the indexer and the pre-construction startup check can both reuse it).

Addressed the best-full-chain half in 71630c3: FullBlockProcessor now owns a package-scoped storage helper, the existing instance method delegates to it, and ExtraIndexer reuses it. I am leaving the pre-construction validity-key lift for a focused cleanup, since it requires changing the HeadersProcessor companion surface rather than this indexer fix.

jozanek · 2026-08-12T15:56:49Z

    * Current newest database schema version. Used to force extra database resync.
    */
-  val NewestVersion: Int = 6
+  val NewestVersion: Int = 7


MINOR Two deliberate operational impacts (both documented in the PR body) should reach the operator-facing release notes: every extraIndex = true node performs a one-time full extra-index rebuild on upgrade to schema 7 (hours on mainnet), and a rollback write failure now shuts down the whole node fail-closed, with a rebuild on restart. Explorer/wallet operators will want to plan for both.

Agreed. @kushti_ru, please carry these two points into the v6.0.4 release notes: existing non-empty schema-6 extra indexes are reset and rebuilt once on schema 7; and a rollback persistence failure now shuts the node down fail-closed, with startup validation/rebuild on restart. Rebuild duration depends on database size and hardware; this PR does not benchmark it.

jozanek · 2026-08-12T15:56:49Z

@@ -150,16 +165,46 @@ class HistoryStorage(indexStore: LDBKVStore, objectsStore: LDBKVStore, extraStor

  def insertExtra(indexesToInsert: Array[(Array[Byte], Array[Byte])],


NIT The log-and-swallow insertExtra/removeExtra wrappers now have zero production callers — everything migrated to the Try variants. Consider deprecating or removing them so new call sites can't silently lose write errors.

Confirmed: there are no in-tree callers left. I am retaining the public wrappers in this maintenance PR to avoid an unannounced source/binary compatibility change. Deprecation can be handled in a cleanup follow-up, with removal reserved for an appropriate breaking release.

jozanek · 2026-08-12T15:56:49Z

  * Base trait for extra indexer actor and its test.
  */
-trait ExtraIndexerBase extends Actor with Stash with ScorexLogging {
+trait ExtraIndexerBase extends Actor with Stash with Timers with ScorexLogging {


NIT ExtraIndexer.scala is now 853 lines, over the repo's 800-line guideline. The rollback/reconcile logic added here is fairly self-contained — a candidate to split into its own file.

Agreed on the guideline breach. I am deferring the split to a dedicated cleanup PR: rollback/reconciliation is coupled to actor state, timers, stash, and self-messages, so extracting it here would widen access boundaries and the review surface of the correctness fix.

jozanek · 2026-08-12T15:56:49Z

    test.lock.unlock()
  }

+  private def cacheBlockTransactions(height: Int, transactions: BlockTransactions): Unit = {


NIT cacheBlockTransactions reaches the private blockCache via reflection on the scalac-mangled name ($$blockCache) — this breaks on a rename or a Scala-version mangling change. Making the cache protected (or adding an explicit test seam like the other overrides in this actor) would be sturdier.

Fixed in 71630c3. The cache remains private; a private[extra] final write seam is used by production prefetch and the same-package test actor, so the reflection and scalac-mangled-name dependency are gone.

kushti · 2026-08-12T17:17:35Z

+    _history.headerIdsAtHeight(height)
+      .find { id =>
+        FullBlockProcessor.isInBestFullChain(historyStorage, id) &&
+          _history.isSemanticallyValid(id) == ModifierSemanticValidity.Valid


the second check looks excessive (if block section with identifier id in the best chain , it is semantically valid)

I traced this before removing it and found a pre-validation window. VerifyADHistorySpecification shows that bestFullBlockOpt can already select a newly stored block while its header and sections are still Unknown, before reportModifierIsValid. typedModifierById filters only Invalid, and ExtraIndexer catch-up can run from startup/retry independently of FullBlockApplied. I therefore kept the explicit Valid check to avoid indexing an Unknown best-chain block.

kushti · 2026-08-12T17:21:03Z

+          unstashAll()
+        case Failure(error) =>
+          log.error(s"Failed to roll back extra indexes to ${targetHeader.height}; shutting down so startup can rebuild", error)
+          requestShutdown()


EXtra indexer should not kill the node in any case

Fixed in 98547b5. A rollback persistence failure now stops only the ExtraIndexer actor via context.stop(self); it no longer shuts down the ActorSystem or node. The durable rollback marker remains for startup rebuild. The regression watches a dedicated actor, proves that actor terminates, proves the ActorSystem stays alive, and preserves the interrupted-marker assertion. Focused suite: 23/23 green.

kushti · 2026-08-15T20:41:29Z

+  }
+
+  protected def blockTransactionsForHeader(header: Header): Option[BlockTransactions] = {
+    history.typedModifierById[BlockTransactions](header.transactionsId).filter(_.headerId == header.id)


.filter is excessive

a-shannon · 2026-08-16T06:47:15Z

Updated in a40c206. I removed the extra .filter: header.transactionsId already identifies the corresponding transaction section, so the typed lookup is sufficient. I kept the separate cache/header check because a cached entry may belong to a different branch. ExtraIndexerSpecification remains green (23/23).

Fix ExtraIndexer checkpoint recovery

6c2ab15

jozanek reviewed Aug 12, 2026

View reviewed changes

a-shannon added 2 commits August 12, 2026 18:17

refactor: tighten extra indexer test seams

71630c3

ci: rerun integration tests

54b86fa

kushti requested changes Aug 12, 2026

View reviewed changes

fix: stop only ExtraIndexer on rollback failure

98547b5

kushti reviewed Aug 15, 2026

View reviewed changes

Remove redundant block transaction filter

a40c206

Retrigger CI after DeepRollBackSpec timeout

893300e

a-shannon mentioned this pull request Aug 18, 2026

Caught-up fix #2472

Merged

kushti deleted the branch ergoplatform:v6.0.4 August 18, 2026 17:17

kushti closed this Aug 18, 2026

a-shannon mentioned this pull request Aug 18, 2026

Resume deferred ExtraIndexer catch-up without reindexing #2476

Merged


		protected def historyStorage: HistoryStorage = _history.historyStorage

		protected def fullChainHeaderAtHeight(height: Int): Option[Header] = {

		@@ -150,16 +165,46 @@ class HistoryStorage(indexStore: LDBKVStore, objectsStore: LDBKVStore, extraStor

		def insertExtra(indexesToInsert: Array[(Array[Byte], Array[Byte])],

Uh oh!

Conversation

a-shannon commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Operational notes

Uh oh!

jozanek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-shannon commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

a-shannon commented Aug 12, 2026 •

edited

Loading