Fail indexer batch on unresolved deactivation reference - #599
Draft
scolear wants to merge 1 commit into
Draft
Conversation
A deactivation event whose activation reference cannot be resolved was persisted with deactivated_event_sequential_id = NULL. Every activeness query anti-joins on that reference, so such a deactivation is invisible and the deactivated contract stays in the Ledger API ACS permanently. Only a warning was logged, and pruning later deletes the orphaned deactivation row while the phantom activation survives, hiding the corruption. Observed in production: a participant served 3 archived contracts as active for 7 weeks, diverging from every other node hosting the same party. Fail the ingestion batch instead and let the indexer restart, following the ReferencedContractNotFoundException recovery pattern. Before the restart, invalidate the cached contract lookups for the contracts whose internal contract id resolved, via a new cache-invalidation-only ContractStore method (contractsPruned would delete live contracts from the in-memory store), so a stale cached mapping cannot cause a restart loop. A persistent resolution failure keeps the indexer restarting: a loud stall is preferable to persisting the corruption. Add an Errors-qualified counter metric for these restarts, and an integrity check that reports deactivation events with a NULL activation reference at or above the pruning point and below the ledger end.
|
✅ All required contributors have signed the CLA for this PR. Thank you! |
Author
|
I have hereby read the Digital Asset CLA and agree to its terms |
Author
|
recheck |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #598.
What
The indexer computes the
deactivated_event_sequential_idback-pointer for each deactivation event at ingestion time. When resolution failed, it logged a warning and persisted the event with a NULL back-pointer. Every Ledger API activeness query anti-joins on that back-pointer, so the archived contract stayed visible as active in the ACS permanently. We observed this in production: a mainnet participant served 3 archived contracts as active for 7 weeks, diverging from every other participant hosting the same party (details in #598 and DA support ticket DA-4793).This PR makes the failure loud instead of corrupting:
dbPreparenow fails the ingestion batch when any deactivation's activation reference stays unresolved, following theReferencedContractNotFoundExceptionrecovery pattern: the indexer restarts and retries. Before throwing, it invalidates the cached contract lookups for the affected contracts, so a stale cached mapping cannot cause a restart loop.refillMissingDeactivatedActivationsbecomes an invariant violation, closing the only code path that wrote NULL back-pointers.Errors-qualified counter,indexer_restart_due_to_unresolved_deactivation, makes the restarts alertable.Design decisions
Cache invalidation uses a new
ContractStore.invalidateCachedContractsmethod instead of the existingcontractsPruned. The existing method deletes contract data on the in-memory store (correct at its call site, where the contracts are proven absent), which would destroy live contracts here. The new method only invalidates caches: the JDBC store invalidates its lookup cache, the in-memory store is a no-op because it has no cache.A persistent resolution failure now stalls the indexer deliberately. The retry converges when the failure was transient (a stale cached mapping, or a ledger-end-cache window). If the activation reference is genuinely unresolvable, the indexer keeps restarting: we consider a loud stall preferable to silently persisting corruption that pruning later makes undiagnosable. If you would rather have an operator escape hatch (bounded retries, or a flag restoring the old warn-and-persist behavior), we are happy to add one — we did not want to grow the config surface unilaterally.
Can a legitimate flow produce an unresolvable deactivation? We believe not. Pruning only removes an activation row together with the deactivation that consumed it, so a live contract's activation row is never pruned; witnessed-only consuming exercises take the
lapi_events_various_witnessedpath and need no back-pointer. An unresolved reference at ingestion time is therefore either transient or evidence of an inconsistent index database. The integration test environments runverifyIntegrityat teardown, so a green CI run across the ACS-import, party-replication, and pruning suites is empirical evidence for this claim — we would appreciate maintainers double-checking the reasoning.How tested
community-common/ledger-api-core(main and test) passscalafmtCheckand compile.ParallelIndexerSubscriptionSpecextended: the throw fromdbPrepare(both failure stages: internal-contract-id resolution miss andlastActivationsmiss), the invalidation contents, the single summarizing error log, the metric increment (viaInMemoryMetricsFactory), the no-invalidation happy path, and the invariant violation throughbatcher.StorageBackendTestsIntegrityextended: the new check reports a NULL back-pointer below the ledger end and at exactly the pruning point, and ignores rows beyond the ledger end. One pre-existing test comment claimed NULL back-pointers are "not reported"; that test only stayed green because the stray-deactivation check fires first, and the comment now says so.ParallelIndexerSubscriptionSpecand the completeStorageBackendSpecH2suite.Not covered locally, deferred to CI: the Postgres storage-backend variant, downstream module compilation, and the integration suites. Two known coverage gaps we chose not to close here because they need the full e2e rig: the production wiring of the invalidation callback is verified by compilation only, and no test pins that the new exception restarts rather than halts the indexer (the sibling
ReferencedContractNotFoundExceptionhas the same gap).User-facing changes
New metric, new integrity check message, and a behavior change (indexer restart instead of silent corruption) — documented in
UNRELEASED.mdunder Bugfixes.