Skip to content

Commit 7bb344b

Browse files
corylanouclaude
andcommitted
fix(directory-watcher): address review feedback for P1 and P2
P1: Fixed directory removal detection to check wasWatchedDir state - os.Stat() fails for deleted directories, leaving isDir=false - Now checks both current state (isDir) and previous state (wasWatchedDir) - Prevents orphaned watches when directories are deleted/renamed P2: Corrected recursive=false semantics to only watch root directory - recursive=false now ignores subdirectories completely (no watches, no replication) - recursive=true watches entire tree recursively - Added TODO to document this behavior on litestream.io - Updated BasicLifecycle test to use recursive=true since it needs subdirectory detection All 9 integration tests pass (129.0s total). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 8e90649 commit 7bb344b

File tree

2 files changed

+38
-22
lines changed

2 files changed

+38
-22
lines changed

cmd/litestream/directory_watcher.go

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -184,19 +184,37 @@ func (dm *DirectoryMonitor) handleEvent(event fsnotify.Event) {
184184
info, statErr := os.Stat(path)
185185
isDir := statErr == nil && info.IsDir()
186186

187+
// Check if this path was previously a watched directory (needed for removal detection)
188+
dm.mu.Lock()
189+
_, wasWatchedDir := dm.watchedDirs[path]
190+
dm.mu.Unlock()
191+
192+
// Handle directory creation/rename
193+
// Note: In non-recursive mode, only the root directory is watched.
194+
// Subdirectories are completely ignored, and their databases are not replicated.
195+
// In recursive mode, all subdirectories are watched and scanned.
187196
if isDir && event.Op&(fsnotify.Create|fsnotify.Rename) != 0 {
188-
if err := dm.addDirectoryWatch(path); err != nil {
189-
dm.logger.Error("add directory watch", "path", path, "error", err)
197+
// Only add watches for: (1) root directory, or (2) subdirectories when recursive=true
198+
if dm.recursive {
199+
if err := dm.addDirectoryWatch(path); err != nil {
200+
dm.logger.Error("add directory watch", "path", path, "error", err)
201+
}
202+
// Scan to catch files created during watch registration race window
203+
dm.scanDirectory(path)
190204
}
191-
// Always scan newly created directory to catch files created during watch registration race window
192-
dm.scanDirectory(path)
193205
}
194206

207+
// Handle directory removal/rename
208+
// Check both current state (isDir) AND previous state (wasWatchedDir)
209+
// because os.Stat fails for deleted directories, leaving watches orphaned
210+
if (isDir || wasWatchedDir) && event.Op&(fsnotify.Remove|fsnotify.Rename) != 0 {
211+
dm.removeDirectoryWatch(path)
212+
dm.removeDatabasesUnder(path)
213+
return
214+
}
215+
216+
// If it's still a directory, don't process as a file
195217
if isDir {
196-
if event.Op&(fsnotify.Remove|fsnotify.Rename) != 0 {
197-
dm.removeDirectoryWatch(path)
198-
dm.removeDatabasesUnder(path)
199-
}
200218
return
201219
}
202220

@@ -309,16 +327,16 @@ func (dm *DirectoryMonitor) matchesPattern(path string) bool {
309327
return matched
310328
}
311329

312-
// scanDirectory walks an existing directory tree and ensures all subdirectories
313-
// are watched while any pre-existing databases are registered. This is used when
314-
// new directories appear before a watch can be established so we do not miss
315-
// files created within them.
330+
// scanDirectory walks a directory to discover pre-existing databases.
331+
// In non-recursive mode, only scans files directly in the directory (subdirectories are ignored).
332+
// In recursive mode, walks the entire tree and adds watches for all subdirectories.
333+
// This is called on startup and when new directories are created to catch files created
334+
// during the fsnotify watch registration race window.
316335
func (dm *DirectoryMonitor) scanDirectory(dir string) {
317-
// In non-recursive mode, only scan the immediate directory (no subdirs)
318-
// In recursive mode, scan the full tree
319-
320336
if !dm.recursive {
321-
// Non-recursive: scan only immediate directory
337+
// Non-recursive mode: Only scan files in the immediate directory.
338+
// Subdirectories and their contents are completely ignored.
339+
// TODO: Document recursive behavior on litestream.io
322340
entries, err := os.ReadDir(dir)
323341
if err != nil {
324342
if !os.IsNotExist(err) {
@@ -328,14 +346,11 @@ func (dm *DirectoryMonitor) scanDirectory(dir string) {
328346
}
329347

330348
for _, entry := range entries {
331-
path := filepath.Join(dir, entry.Name())
349+
// Skip subdirectories - they're not watched in non-recursive mode
332350
if entry.IsDir() {
333-
// Watch subdirectories but don't recurse into them
334-
if err := dm.addDirectoryWatch(path); err != nil {
335-
dm.logger.Error("add directory watch", "path", path, "error", err)
336-
}
337351
continue
338352
}
353+
path := filepath.Join(dir, entry.Name())
339354
dm.handlePotentialDatabase(path)
340355
}
341356
return

tests/integration/directory_watcher_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import (
1818
func TestDirectoryWatcherBasicLifecycle(t *testing.T) {
1919
RequireBinaries(t)
2020

21-
db := SetupDirectoryWatchTest(t, "dir-watch-lifecycle", "*.db", false)
21+
// Use recursive:true because this test creates databases in subdirectories (tenant1/app.db, etc.)
22+
db := SetupDirectoryWatchTest(t, "dir-watch-lifecycle", "*.db", true)
2223

2324
// Create config with directory watching
2425
configPath, err := db.CreateDirectoryWatchConfig()

0 commit comments

Comments
 (0)