fullhistory: build empty cold indexes for zero-key coverages (closes #826)#858
Merged
tamirms merged 1 commit intoJul 13, 2026
Merged
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
tamirms
force-pushed
the
fix/fullhistory-826-empty-txhash-index
branch
from
July 13, 2026 14:12
d362b06 to
b42b8d8
Compare
chowbao
reviewed
Jul 13, 2026
| github.com/spf13/pflag v1.0.5 | ||
| github.com/stellar/go-stellar-sdk v0.6.1-0.20260618191317-308407eca8c6 | ||
| github.com/stellar/streamhash v0.0.0-20260622155330-9e08e05c5fa6 | ||
| github.com/stellar/streamhash v0.0.0-20260713140807-9f6eeb7acecf |
Contributor
There was a problem hiding this comment.
TODO: bump when stellar/streamhash#12 (review) is merged (one possible change in the PR which would change the commit hash)
chowbao
approved these changes
Jul 13, 2026
…826) streamhash now accepts zero-key builds (stellar/streamhash#12), so the cold stores no longer special-case an empty coverage. txhash (the #826 stall): BuildColdIndex drops both ErrEmptyBuildSet returns and builds a valid empty index for a zero-key coverage (no inputs, or inputs with no entries — a window of only zero-transaction chunks). buildTxhashIndex then commits normally, so an all-empty window freezes instead of re-planning every restart. Removes the #826 TODO at resolveWindow. events: drop the zero-length index.hash sentinel. WriteColdIndex builds a real streamhash empty index over zero terms (deleting writeEmptyColdIndex); openMPHF always opens it; isEmpty is NumKeys()==0. Both stores now handle empty identically. go.mod points streamhash at the unmerged PR #12 commit for development; re-pin to a merged commit/tag before landing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tamirms
force-pushed
the
fix/fullhistory-826-empty-txhash-index
branch
from
July 13, 2026 16:57
b42b8d8 to
7d344c5
Compare
Contributor
Author
|
@codex[agent] review |
|
To use Codex here, create a Codex account and connect to github. |
Contributor
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
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
2 participants
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.
Summary
Closes #826. A window of only zero-transaction chunks stalled the full-history daemon: the txhash cold index build returned
ErrEmptyBuildSet, so the coverage stayedfreezingand the daemon re-planned the same window on every restart. streamhash now accepts zero-key builds (stellar/streamhash#12), so both cold stores build a real empty index for a zero-key coverage instead of special-casing it.What changed
txhash (the stall).
BuildColdIndexdrops bothErrEmptyBuildSetreturns and builds a valid empty index for a zero-key coverage (no inputs, or inputs that hold no entries). It skips the k-way merge when there is nothing to feed.buildTxhashIndexthen commits normally, so an all-empty window freezes instead of re-planning every restart. Removed theTODO(#826)atresolveWindow.events (fixing both stores consistently). The events store already tolerated empty chunks via a zero-length
index.hashsentinel, so it was not stalling, but this migrates it off the sentinel so both stores handle empty identically.WriteColdIndexbuilds a real streamhash empty index over zero terms and writes a zero-recordindex.packthrough the ordinary path;writeEmptyColdIndexis deleted,openMPHFalways opens the file, andisEmpty()isNumKeys() == 0.Testing
New
TestBuildTxhashIndex_AllEmptyWindowFreezesproves the backfill flow (an all-empty window freezes rather than stalling), and the six tests that encodedErrEmptyBuildSetor the zero-length sentinel were reworked to the real-empty-index behavior, including asserting the empty txhash index carries correct[minLedger, maxLedger]coverage metadata. All green: txhash, eventstore, backfill, lifecycle, and the fullTestE2E_DaemonLifecycle. Build, vet, and gofmt clean. The change deletes more than it adds, since the sentinel andErrEmptyBuildSetmachinery is gone.Two dependencies before this can merge
go.modpoints streamhash at that PR's commit so this builds and tests now. Re-pin it to a merged commit on streamhashmainbefore landing.openMPHF,isEmpty, and the cold-reader empty cross-check, which fullhistory: hardening + contract notes from the pre-existing-code review (closes #840) #847 also touches. Whichever lands second needs a rebase, so sequence the two deliberately.🤖 Generated with Claude Code