Skip to content

fix(index): resume interrupted full indexing - #120

Closed
Hvizeu wants to merge 2 commits into
tumourlove:masterfrom
Hvizeu:codex/fix-resumable-full-index
Closed

fix(index): resume interrupted full indexing#120
Hvizeu wants to merge 2 commits into
tumourlove:masterfrom
Hvizeu:codex/fix-resumable-full-index

Conversation

@Hvizeu

@Hvizeu Hvizeu commented Jul 30, 2026

Copy link
Copy Markdown

What and why

Full-project indexing currently starts over after an editor shutdown or crash because an interrupted run never writes last_full_index. On large projects, repeated restarts can prevent the initial index from ever completing.

This change adds a schema-v3 recovery lifecycle that:

  • stores per-asset progress by package path, saved hash, and indexer name
  • commits indexed data and its progress checkpoint in the same SQLite transaction
  • refreshes metadata after a restart, removes deleted packages, and queues only unfinished or hash-invalidated work
  • tracks post-pass completion and reruns invalidated passes when indexed assets change
  • processes animation assets individually on compiler-idle editor ticks so interrupted animation indexing can resume
  • requires all registered full-index post-passes to complete before publishing last_full_index
  • leaves failed work pending and rolls back incomplete transactions
  • preserves the existing forced-reindex behavior, which intentionally clears data and recovery state

Checklist

  • Builds on UE 5.7 (the compile floor)
  • Builds on UE 5.8
  • Any engine API newer than 5.7 sits behind an ENGINE_MINOR_VERSION gate, with a working 5.7 path (N/A: no newer engine API added)
  • No unrelated whitespace, line-ending, or reformatting churn in the diff
  • Specs updated (Docs/specs/SPEC_MonolithIndex.md)
  • CHANGELOG.md updated under ## [Unreleased]
  • Recovery behavior verified with editor automation on both supported engine versions
  • New or changed actions verified in-editor (N/A: no actions changed)

Testing notes

  • git diff --check upstream/master passes.
  • Monolith.Index.Recovery.CheckpointTransaction, Monolith.Index.Recovery.Lifecycle, and Monolith.Index.Recovery.PostPassCleanup pass under UE 5.7: 3 succeeded, 0 failed.
  • The same three tests pass under UE 5.8: 3 succeeded, 0 failed.
  • UE 5.7: Editor Win64 Development completed the final 49-action build successfully, including the changed Monolith sources, tests, dependent plugin modules, and DLL links.
  • UE 5.8: Editor Win64 Development completed the final 23-action build successfully, including the changed Monolith sources, tests, dependent plugin modules, and DLL links.
  • The remaining compiler warnings are pre-existing deprecations or warnings outside the changed code.

@Hvizeu
Hvizeu marked this pull request as ready for review July 30, 2026 17:25
tumourlove added a commit that referenced this pull request Aug 1, 2026
A full index wiped the database at the start and wrote its completion marker
only at the very end, so any crash, kill or power loss left no marker -- and
the next launch began another full index, which immediately erased every
batch the previous run had finished. On a large project one crash cost hours;
repeated crashes meant the index could never complete at all.

Schema v3 adds deep_indexed_hash and deep_index_attempts to assets, plus an
in_progress marker in meta. Migration is additive and leaves existing rows
untouched; a v0.21.3 build reading a v3 database is inert rather than broken.
Force-reindex still wipes, which is its documented purpose.

Two records with deliberately DIFFERENT durability requirements, which is the
whole mechanism and is easy to get backwards:

The deep-index checkpoint commits in the SAME transaction as the data it
vouches for, so a rollback can never leave a checkpoint pointing at rows that
are not there.

The attempt counter commits in its OWN transaction, opened and committed
BEFORE the batch work transaction. This is what makes the poison pill work. A
crash mid-asset must leave evidence behind, and a statement inside an open
transaction is not durable -- SQLite runs a DELETE journal here, so the crash
would roll the marker back along with the work and the counter would still
read zero on resume. The asset that killed the editor would then be re-queued
forever: a crash-every-launch loop, strictly worse than the bug being fixed.
The frame-budget commit does not cover it either, since it only fires after
at least one asset has been processed and so never protects the FIRST asset
of a batch -- exactly where a resumed queue puts the poison asset. After
repeated interrupted attempts an asset is skipped, logged, and surfaced
through project get_stats rather than retried indefinitely.

The completion gate tolerates per-asset failures. Making last_full_index
conditional on a zero error count would reproduce this very issue from the
opposite direction: deep-index errors are routine on real projects (assets
referencing classes from disabled plugins, redirector stubs) and the codebase
already carries an SEH guard because some animation assets crash on load. One
such asset would mean the marker is never written and every launch runs a full
index. Only structural failures fail the gate now.

bDeferFirstTimeIndex is honoured on the resume path. It is the escape hatch
for the GC worker-context crash class, so bypassing it when resuming would
strip it at the exact moment a user needs it.

Also routes a non-force monolith_reindex to the resume path. Without last_full_index
an interrupted run cannot pass the incremental check, so it fell through to
StartFullIndex and reset -- and since that action is how an agent typically
kicks the index after a crash, it silently destroyed the progress this change
exists to preserve.

Reported and diagnosed by @Hvizeu (#117), whose PR #120 established the
lifecycle design this follows.
@tumourlove

Copy link
Copy Markdown
Owner

Shipped in v0.22.0. I land contributor fixes as my own commits rather than merging the branch — I keep the shipped history single-author for release integrity, and credit you in the release notes instead. No reflection on the patch.

Your diagnosis was exactly right and I verified every link: StartFullIndex() unconditionally resetting, last_full_index written only at the end, ShouldAutoIndex() therefore restarting and wiping the recoverable progress. The schema-v3 lifecycle and the atomic-checkpoint idea are yours and are preserved.

I did land it smaller, and there were three things I changed rather than took:

The poison pill. This is the important one. Your version records completions but never attempts, so a hard crash leaves no trace and the resume re-queues the asset that killed the editor — a crash-every-launch loop, which is worse than the bug. It is made sharper by the bDeferFirstTimeIndex bypass on the resume path: that flag is the documented escape hatch for exactly this crash class, and overriding it in the crash case leaves deleting ProjectIndex.db by hand as the only recovery. The shipped version counts attempts, skips after repeated failures, and surfaces the skipped asset through project get_stats.

Subtlety worth recording, because I got it wrong first and a review caught it: the attempt marker has to commit in its own transaction before the batch work. Written inside the batch transaction it is not durable — SQLite runs a DELETE journal here, so the crash rolls the marker back along with the work and the counter still reads zero on resume. The pill can never fire in the one case it exists for.

The completion gate. Errors > 0 || DeepErrors > 0 blocking last_full_index reproduces #117's symptom from the other direction. DeepErrors++ fires routinely — assets referencing classes from disabled plugins, redirector stubs — and the codebase already carries an SEH guard specifically because some animation assets crash on load. One such asset and the marker is never written, so every launch runs a full index. Only structural failures fail the gate now.

The animation indexer. Per-asset transactions and BatchSize = 1 are a throughput regression bundled into a correctness fix, so I kept the existing dispatch model.

Also not taken: post-pass resume checkpoints (they self-invalidate on the common interruption case), the ClearFullIndexPostPassData name→SQL map (wrong layer, and wrong for AI), and activating the three dead sentinel indexers — that last one is a real latent bug you surfaced, but it changes the runtime and memory profile of every full index, so it gets its own change and its own verification rather than riding along. It is logged.

One note on the checklist: the three automation tests exercise FMonolithIndexDatabase in isolation — none of them drives an interrupted index, a resume, or a crash. The shipped tests include a rollback-durability assertion, because "the counter increments" cannot distinguish the durable implementation from the broken one.

@tumourlove tumourlove closed this Aug 1, 2026
kunkunGames pushed a commit to kunkunGames/monolith-fork that referenced this pull request Aug 3, 2026
A full index wiped the database at the start and wrote its completion marker
only at the very end, so any crash, kill or power loss left no marker -- and
the next launch began another full index, which immediately erased every
batch the previous run had finished. On a large project one crash cost hours;
repeated crashes meant the index could never complete at all.

Schema v3 adds deep_indexed_hash and deep_index_attempts to assets, plus an
in_progress marker in meta. Migration is additive and leaves existing rows
untouched; a v0.21.3 build reading a v3 database is inert rather than broken.
Force-reindex still wipes, which is its documented purpose.

Two records with deliberately DIFFERENT durability requirements, which is the
whole mechanism and is easy to get backwards:

The deep-index checkpoint commits in the SAME transaction as the data it
vouches for, so a rollback can never leave a checkpoint pointing at rows that
are not there.

The attempt counter commits in its OWN transaction, opened and committed
BEFORE the batch work transaction. This is what makes the poison pill work. A
crash mid-asset must leave evidence behind, and a statement inside an open
transaction is not durable -- SQLite runs a DELETE journal here, so the crash
would roll the marker back along with the work and the counter would still
read zero on resume. The asset that killed the editor would then be re-queued
forever: a crash-every-launch loop, strictly worse than the bug being fixed.
The frame-budget commit does not cover it either, since it only fires after
at least one asset has been processed and so never protects the FIRST asset
of a batch -- exactly where a resumed queue puts the poison asset. After
repeated interrupted attempts an asset is skipped, logged, and surfaced
through project get_stats rather than retried indefinitely.

The completion gate tolerates per-asset failures. Making last_full_index
conditional on a zero error count would reproduce this very issue from the
opposite direction: deep-index errors are routine on real projects (assets
referencing classes from disabled plugins, redirector stubs) and the codebase
already carries an SEH guard because some animation assets crash on load. One
such asset would mean the marker is never written and every launch runs a full
index. Only structural failures fail the gate now.

bDeferFirstTimeIndex is honoured on the resume path. It is the escape hatch
for the GC worker-context crash class, so bypassing it when resuming would
strip it at the exact moment a user needs it.

Also routes a non-force monolith_reindex to the resume path. Without last_full_index
an interrupted run cannot pass the incremental check, so it fell through to
StartFullIndex and reset -- and since that action is how an agent typically
kicks the index after a crash, it silently destroyed the progress this change
exists to preserve.

Reported and diagnosed by @Hvizeu (tumourlove#117), whose PR tumourlove#120 established the
lifecycle design this follows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants