Skip to content

Watch directories created before their parent's watch landed - #1558

Merged
thekevinbot merged 5 commits into
mainfrom
bot/1557-watch-rename-1102
Nov 2, 2026
Merged

thekevinbot merged 5 commits into
mainfrom
bot/1557-watch-rename-1102

Conversation

@thekevinbot

@thekevinbot thekevinbot commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1557

On inotify each directory gets its own watch. When a directory appears, the watcher walked it once and then watched what the walk found. A subdirectory created after that walk but before its parent's watch landed got no event and was never watched, so every later write inside it was lost. mkdir -p a/b/c racing the watcher's thread (easiest to hit pinned to one CPU) does exactly that.

Red commit first: watch_tree_watches_a_directory_created_before_its_parent_was_watched (unit test, packages/rust/src/watcher.rs).

The fix: watch_tree walks the new directory again after watching what it found, and stops once a walk finds nothing unwatched. A walk that finds nothing new costs one extra read of the new tree.

Hand test (dirsql server, all processes pinned to one CPU, mkdir -p dN/a/b/c/d/e/f/g then write r.json at the bottom, 200 times per run, 4 runs): before, 3 of 800 writes produced no event; after, 0 of 800.

E2E Verification

  • Ran e2e suites locally for every affected SDK
  • Python SDK e2e: watch_speed, jsonl_parse_speed, on_file_query, fanout -- 7 passed
  • TypeScript SDK e2e: pnpm test:e2e -- passed
  • Rust core e2e (if applicable): cargo test -p dirsql --features cli -- all passed
  • packages/python/e2e-attestations/<branch>.json receipt written if packages/python changed (just e2e-attest-python)
  • packages/ts/e2e-attestations/<branch>.json receipt written if packages/ts changed (just e2e-attest-ts)
  • Command(s) run: testing-conventions e2e attest 'uv run python -m pytest tests/e2e/watch_speed_test.py tests/e2e/jsonl_parse_speed_test.py tests/e2e/on_file_query_test.py tests/e2e/fanout_test.py -q'; testing-conventions e2e attest 'pnpm test:e2e'
  • Result summary: pass. watch_speed_test is flaky around its 1.1x bar on this host with and without this change (unfixed core failed 3 of 3, this branch passed 2 of 3); it measures writes to an existing file, a path this change does not touch.

Changelog / Migrations

  • Changelog fragment added under <root>/<pkg>/changelog.d/ for each changed package (or: skip-changelog trailer on a commit with reason)
  • Migration fragment with the same slug added under <root>/<pkg>/migrations.d/ for each changed package (every subsection but Summary _None._ when nothing breaks)

@thekevinbot
thekevinbot merged commit d7e11e8 into main Nov 2, 2026
89 checks passed
@thekevinbot
thekevinbot deleted the bot/1557-watch-rename-1102 branch November 2, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

watch() misses a renamed-in file under CPU contention

1 participant