Repository navigation
fix: preserve pending data when recovery validation fails - #280
Open
perfloop[bot] wants to merge 2 commits into
Open
perfloop[bot] wants to merge 2 commits into
perfloop[bot] wants to merge 2 commits into
Conversation
|
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
perfloop
Bot
force-pushed
the
perfloop-pr-open-1b34gar7w6
branch
from
October 5, 2026 13:45
287d3ec to
f60f7c8
Compare
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.
Startup recovery now preserves pending raw-file data when a metadata point read or file stat fails for an operational reason. It returns the validation error instead of treating an unproven check as grounds for deletion. Confirmed missing metadata, ENOENT, and size-mismatch cleanup remain unchanged.
Recovery aggregates validation errors across batches while allowing unrelated, confirmed cleanup to finish. If recovery fails while
NewStorageWithConfigis starting up, it closes the segment manager and metadata database before returning, so a caller can retry in the same process. No public API or on-disk schema changes.TestRecoveryValidationFailureIsNonDestructiveinjects metadata-read and non-ENOENT stat errors separately, checks the returned error and preserved metadata, compaction row, and file bytes, then clears the fault and verifiesStorage.Get. Its 101-entry case also checks both errors across the 100-result batch boundary. Cleanup controls cover missing metadata, ENOENT, and size mismatch; other tests verify unrelated confirmed cleanup and that failed startup releases the database lock. The fault-reach checks now fail if an injected path is not exercised instead of skipping the assertions.The focused and full storage tests, race suite, selected cache-warming workflow integration test, all-target build, lint, and license checks passed.
The same assertion was violated on comparison revision
eaaa3f8e92d3b6d9a2c36406a185c87723a32f23and satisfied on the final source.Recorded proof ran with retained support that is not included in this pull request.
An earlier unfiltered integration run timed out at
tests/integration: TestIntegration_Objects; the selected cache-warming workflow case passed. Full repository CI remains pending. Production frequency of these injected failure types is unmeasured.Generated by Perfloop. Measurements and checks. Reproduce this comparison (tenant access required).
Note
Medium Risk
Changes startup recovery deletion semantics and storage init teardown on failure; incorrect inconclusive handling could leave stale compaction rows, but the shift reduces risk of data loss on transient I/O errors.
Overview
Startup recovery no longer deletes pending raw-file metadata, compaction rows, or on-disk bytes when validation cannot be proven—metadata point-read I/O failures and non-
ENOENTstat errors are treated as inconclusive, left untouched, and surfaced as aggregated errors instead of being classified as corrupted/missing.Confirmed cleanup behavior is unchanged for orphaned metadata, genuine missing files, and size mismatches. Unrelated entries that validate conclusively still get cleaned up in the same run. If recovery fails during
NewStorageWithConfig, the constructor now closes the segment manager and metadata DB so the process can retry without holding the RocksDB lock.Injectable
getMetadata/statFilehooks and external tests cover non-destructive failure paths, cross-batch error joining, constructor retry, and cleanup controls.Reviewed by Cursor Bugbot for commit f60f7c8. Bugbot is set up for automated code reviews on this repo. Configure here.