feat: support zero-key (empty) indexes#12
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes the builder contract to treat totalKeys == 0 as a valid “empty index” build result (instead of returning ErrEmptyIndex), ensuring empty windows can be built without caller-side special casing while preserving query semantics (QueryRank/QueryPayload miss with ErrNotFound).
Changes:
- Allow
NewSortedBuilder/NewUnsortedBuilderto accepttotalKeys == 0, forcing single-threaded completion for that case. - Prevent an unsorted-build nil dereference by emitting the empty index directly when no keys were ever added.
- Remove
ErrEmptyIndexand add/adjust tests to pin the new empty-index behavior across algorithms/build modes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test_helpers_test.go | Removes helper-level “empty input” guards so tests can build empty indexes. |
| sentinels.go | Removes exported ErrEmptyIndex sentinel from the public API surface. |
| internal/sherr/errors.go | Deletes the internal ErrEmptyIndex definition. |
| error_test.go | Updates TestBuilderZeroKeys to assert empty-index success + ErrNotFound query behavior. |
| empty_index_test.go | Adds comprehensive empty-index coverage across algorithms, builder types, and payload layouts (plus WithWorkers). |
| builder.go | Removes the totalKeys==0 guard, forces single-threaded empty builds, and extracts shared empty-block finalize helper. |
| builder_unsorted.go | Adds the zero-key unsorted Finish() fast path to avoid nil unsortedBuf dereference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove the newBuilder guard that rejected totalKeys == 0. numBlocks() floors at 2, so a zero-key build now flows through the existing empty-block machinery and produces a valid index whose every QueryRank returns ErrNotFound. Force single-threaded for a zero-key build (nothing to parallelize) and guard the unsorted Finish, whose lazily-created buffer is nil when no keys were ever added. Remove ErrEmptyIndex: zero keys is no longer an error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tamirms
force-pushed
the
feat/empty-index-support
branch
from
da9a38b to
9f6eeb7
Compare
tamirms
added a commit
to stellar/stellar-rpc
that referenced
this pull request
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>
chowbao
approved these changes
Jul 13, 2026
Drop the ErrEmptyIndex row from the §7.5 error reference (the error was removed) and note that N == 0 builds a valid empty index whose queries return ErrNotFound. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tamirms
added a commit
to stellar/stellar-rpc
that referenced
this pull request
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>
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.
Summary
NewSortedBuilderandNewUnsortedBuildernow accepttotalKeys == 0and produce a valid empty index whose everyQueryRankreturnsErrNotFound, instead of failing withErrEmptyIndex. A caller with no keys no longer needs a special case.Motivation
A downstream cold-index build over a zero-key window (stellar/stellar-rpc#826) returned
ErrEmptyIndex, so the caller had to special-case the empty result. When a coverage window contained only empty chunks, that special case stalled the daemon. Treating a zero-key index as a first-class empty result lets callers build unconditionally.What changed
The on-disk format already tolerated this: the header permits a zero-key index, and the query path returns
ErrNotFoundfor every key when no block holds a key. The only blockers were an artificial guard and one lazy-init hazard.totalKeys == 0guard innewBuilder.numBlocks()floors at 2, so a zero-key build flows through the existing empty-block machinery (commitEmptyBlock) and finalizes two empty blocks; every lookup misses.Finishemits the empty index directly when its lazily-created buffer was never initialized (no keys added), becausefinishUnsorteddereferences that buffer immediately and would otherwise panic.commitTrailingEmptyBlocksAndFinalize, shared by the sorted single-threaded finish and the unsorted zero-key path.ErrEmptyIndex, which is no longer returned. This is a clean break, appropriate pre-1.0.Testing
TestEmptyIndex: both algorithms, both builder types, and mphf-only versus the payload-plus-fingerprint layout, asserting the build succeeds, the index opens, passes integrityVerify(), reports zero keys, and everyQueryRankandQueryPayloadreturnsErrNotFound.TestEmptyIndexWithWorkerspins that a zero-key build requested withWithWorkersstays off the parallel finish paths.TestBuilderZeroKeysfor the new contract.go vet,golangci-lint, and the correctness, corrupt-input, and parallel-pipeline fuzzers all pass.🤖 Generated with Claude Code