Skip to content

Fix ETCD service watcher discovery - #71

Merged
billy-guo merged 2 commits into
mainfrom
bguo/fix-etcd-routing-shortlist
Jun 30, 2026
Merged

Fix ETCD service watcher discovery#71
billy-guo merged 2 commits into
mainfrom
bguo/fix-etcd-routing-shortlist

Conversation

@billy-guo

@billy-guo billy-guo commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Description

  • Updated _AsyncHashRing and AsyncServiceWatcher to iterate over each event generated from EtcdWatcher.continue_watching()
  • Deleted _blocking_next function which only consumed the first event

Osprey workers are unable to discover new pods through ETCD because we only take the first event from watcherd and then abandon the generator. As a result, we lose track of the ongoing stream of updates, so when new pods declare themselves on ETCD, the workers never receive that update.

Checklist

  • Tests pass locally
  • uv run ruff check . passes (no unused imports or other lint errors)
  • uv tool run fawltydeps --check-unused --pyenv .venv passes (no unused dependencies)
  • Updated CHANGELOG.md with my changes, if applicable

@billy-guo
billy-guo requested a review from haileyok as a code owner June 29, 2026 17:24
@billy-guo
billy-guo marked this pull request as draft June 29, 2026 20:37
@billy-guo
billy-guo force-pushed the bguo/fix-etcd-routing-shortlist branch from 479dbe7 to e9282d9 Compare June 29, 2026 20:38
…tances

The async etcd discovery watch loop consumed continue_watching() one event at
a time via run_in_executor, recreating the generator per event. That reset its
resume index and dedup mux and defeated the generator's documented infinite,
self-healing recovery (timeouts / event_index_cleared / transient errors). On
shortlist pod turnover an empty FullSyncRecursive (key-deleted / index_cleared)
wiped a worker's instance set, which then never repopulated, so select() raised
"No service for smite_shortlist" forever until the worker (not the shortlist)
was restarted.

Drive a single persistent continue_watching() generator instead, in the
_watch_loop of both AsyncServiceWatcher (ROUND_ROBIN / shortlist) and
_AsyncHashRing (SCALAR, same latent bug):
  - the generator is created once and pumped each iteration with
    run_in_executor(None, lambda: next(it)), never recreated per event, so its
    resume index and dedup mux are preserved;
  - events are applied on the loop thread, keeping instance/ring mutation
    single-threaded and consistent with select() and the DOWN listener's
    create_task;
  - the generator is reopened only on StopIteration, and on error it is logged
    and reopened after exponential backoff (1s -> 30s).

This mirrors the sibling AsyncEtcdSourcesProvider._watch_loop and stays on the
existing run_in_executor model: no per-watcher OS threads, no cross-thread
queue, and stop() remains a deterministic task cancel/await.

No empty-sync guard: continuous consumption lets the set self-repopulate via
incremental upserts, and a guard would break scale-to-zero and could retain
stale dead-pod entries.

Adds test_async_directory.py covering recovery-after-empty-full-sync, stop()
teardown, and the ring recovery path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@billy-guo
billy-guo force-pushed the bguo/fix-etcd-routing-shortlist branch from e9282d9 to bbce9ee Compare June 29, 2026 20:59
@billy-guo
billy-guo force-pushed the bguo/fix-etcd-routing-shortlist branch from 299ff8e to 8eab176 Compare June 29, 2026 22:47
@billy-guo
billy-guo marked this pull request as ready for review June 30, 2026 13:08

@cmttt cmttt left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh this makes sense

@billy-guo
billy-guo merged commit a8804c2 into main Jun 30, 2026
2 of 8 checks passed
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.

2 participants