Repository navigation
Fix stale file tracking after monitored directory removal - #236
Open
bob sira (bobsira) wants to merge 1 commit into
Open
bob sira (bobsira) wants to merge 1 commit into
bob sira (bobsira) wants to merge 1 commit into
Conversation
Process removal notifications regardless of the file filter and clear tracked descendants and their path and file-ID mappings. Add regression coverage for directory moves, short paths, and sibling preservation. Related to #219.
There was a problem hiding this comment.
🟡 Changes recommended
Move-back monitoring, mixed-path cleanup, and bulk-removal performance issues remain unresolved.
3 open issues
What changed in this PR
This PR addresses the reproduced directory-removal case related to #219 by clearing stale file tracking when a monitored subdirectory leaves the watched tree. It does not claim to resolve the reporter’s ordinary remote-file deletion scenario.
Changes:
- Process removal notifications regardless of the file-name filter and clean up descendant tracking entries.
- Add integration and short-path regression tests, with access to internal tracking state.
- Document monitoring behavior after directory removal.
| File | Description |
|---|---|
| LogMonitor/src/LogMonitor/LogFileMonitor.h | Grants the test class access to internal tracking state. |
| LogMonitor/src/LogMonitor/LogFileMonitor.cpp | Bypasses filtering for removals and clears descendants from tracking maps. |
| LogMonitor/LogMonitorTests/LogFileMonitorTests.cpp | Adds directory-removal, short-path, and sibling-preservation coverage. |
| LogMonitor/docs/README.md | Documents directory-removal behavior. |
Review effort: Balanced
Comment on lines
+1144
to
+1146
| if (matchesRemovedPath(it->first)) | ||
| { | ||
| m_longPaths.erase(longPathIterator); | ||
| it = m_logFilesInformation.erase(it); |
Comment on lines
+1156
to
+1157
| if (matchesRemovedPath(it->first) || | ||
| matchesRemovedPath(it->second) || |
Comment on lines
+1154
to
+1158
| for (auto it = m_longPaths.begin(); it != m_longPaths.end();) | ||
| { | ||
| if (matchesRemovedPath(it->first) || | ||
| matchesRemovedPath(it->second) || | ||
| m_logFilesInformation.find(it->second) == m_logFilesInformation.end()) |
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
Related to #219. This fixes a reproduced directory-removal case; it does not claim that every cause of the original report is resolved.
When a monitored subdirectory moves outside the watched tree, Windows can report removal of the directory without individual removal notifications for its descendants. The file-name filter previously discarded directory notifications such as
movewhen configured for*.txt. The removal handler also only removed an exact file match. Descendants remained tracked and were opened repeatedly by the polling loop, with missing-path errors suppressed.Validation
mainatd07608dwith the candidate on AKS 1.34.11 / Windows Server 2022 using an Azure Files SMB share. Monitor pods ran on one Windows node, with the deletion client on a second node.The Azure ETW capture lost zero events, with at least 66 seconds of post-deletion observation for the candidate. Both builds continued logging active and similarly named sibling files; the candidate read all four recreated files. Neither build opened the excluded
.bincontrol file. All temporary Azure resources were deleted and deletion was verified.An additional local comparison using the signed released v2.0 executable, unmodified baseline, and candidate did not reproduce ordinary or burst individual-file deletion retries (512 files per build, 16-way deletion concurrency, 70-second observation, zero trace-event loss).
Reporter validation
The reported ordinary remote-file deletion scenario remains unconfirmed. Keep #219 open pending validation with the reporter's actual retention/deletion operation. Comparing their existing executable with both the unmodified baseline and candidate will distinguish this patch from other changes since v2.0.