Repository navigation
fix(windows): avoid orphaning mapped index generations on NTFS - #156
Merged
Merged
Conversation
Retain replaced index files in uniquely named generations and use non-POSIX deletion on Windows. Release merge readers before cleanup, retry retired generations on startup and idle ticks, and preserve uncommitted backups for recovery. Co-authored-by: Copilot App <[email protected]>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Windows-specific unsafe filesystem operations and crash-recovery semantics warrant final human validation.
Review tier: Balanced
Findings: None
What changed in this PR
Prevents Windows index publications from orphaning mapped generations in NTFS’s $Deleted namespace.
Changes:
- Adds retryable retired-generation cleanup using non-POSIX Windows deletion.
- Releases mapped readers before publication and preserves failed rollback backups.
- Documents behavior and adds focused cleanup/publication regression tests.
| File | Description |
|---|---|
tgrep-core/src/hybrid.rs |
Clarifies mapped-generation lifecycle requirements. |
tgrep-cli/src/serve/index_cleanup.rs |
Implements safe cleanup and retry logic. |
tgrep-cli/src/serve.rs |
Integrates retirement, cleanup, rollback, and tests. |
README.md |
Documents Windows cleanup behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Prevent index publication from force-unlinking memory-mapped index generations into NTFS's
$Deletednamespace..retiredgenerations, separate from reusable staging directories.Root cause
Both incremental merge and checkpoint paths retained the old reader through publication, while publication recursively deleted its backup files. Rust's Windows filesystem operations can fall back to POSIX deletion/replacement when mapped files reject ordinary deletion.
A regression on Windows reproduced the old
index.binhandle changing toC:\$Extend\$Deleted\.... The updated path retains the generation under its original volume until Windows accepts non-POSIX deletion, including when an independent reader still maps it.Platform behavior
The reported persistent NTFS
$Deletedaccumulation is Windows-specific. Linux and macOS can temporarily retain storage for an unlinked file while a handle or memory mapping remains alive; they normally reclaim it after the last reference closes. Their standard filesystem deletion behavior is unchanged.This prevents new orphaned generations; it does not reclaim storage already stranded by an older build.
Validation
On Windows: 22 focused cleanup/publication/rollback regressions pass, 254 active CLI unit tests pass (one existing ignored), and 10 indexed-file/watcher integration tests pass. Targeted Clippy and rustfmt pass. Coverage includes multiple retained generations, independent readers, reloads, rejected publication, cleanup retry, and rollback without replacing a mapped file. Linux/macOS execution remains covered by the existing CI matrix.