Skip to content

Discovery sentinel drops worker-updated events carrying changed metadata — Closes #292#303

Merged
conradbzura merged 2 commits into
wool-labs:releasefrom
conradbzura:292-fix-dropped-worker-updates
Jul 14, 2026
Merged

Discovery sentinel drops worker-updated events carrying changed metadata — Closes #292#303
conradbzura merged 2 commits into
wool-labs:releasefrom
conradbzura:292-fix-dropped-worker-updates

Conversation

@conradbzura

@conradbzura conradbzura commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

WorkerMetadata hashes by uid alone while the generated dataclass equality compares every field, so a pool keyed by the metadata record could not address a worker whose record had changed. Every membership test failed for exactly the events that mattered: the discovery sentinel dropped worker-updated events carrying changed metadata, a worker-dropped event whose record differed from the cached one missed its entry, and a re-announcement inserted a duplicate that consumed a second lease slot.

Key the pool by the worker's uid, which is what actually identifies a worker. LoadBalancerContext maps uid to the worker's current record and connection, so membership, lookup, and eviction all follow from the key, the mutators become single dict operations, and a duplicate entry is no longer possible to express. The lease cap counts distinct uids, so a worker re-announcing itself refreshes its entry rather than being turned away at capacity.

The second commit applies the same move to the balancer protocol, and fixes a defect the first one exposes rather than introduces. A balancer yielded the (metadata, connection) pair it had read from the pool, and the proxy dispatched through whatever pair it was handed — but a balancer selects from a view it may have read before a refresh replaced a worker's entry, so that pair could name a superseded address. delegate now yields the worker's uid, and the proxy resolves that name against the live pool at the moment it dispatches. A balancer cannot route a task to a superseded address however stale the view it chose from, because the record it read is not the record dispatched through.

Closes #292

Proposed changes

Key the worker pool by uid

LoadBalancerContext stores dict[uuid.UUID, tuple[WorkerMetadata, WorkerConnection]]. Presence is uid in workers, resolution is workers[uid], and the three mutators are one dict operation each. add_worker admits or replaces; update_worker refreshes what is already admitted; remove_worker evicts. The upsert flag is gone — the two verbs carry two contracts and no longer need a boolean to tell them apart.

LoadBalancerContextView exposes the uid-keyed mapping and nothing else. Its member set is unchanged, so no existing context implementation breaks its isinstance conformance. The mapping's key type does change, which is breaking for a custom balancer that iterates context.workers.items().

Connections displaced by a refresh are deliberately left unclosed. A connection is a lazy handle, and the channel it names is owned by the pooling layer, which reaps it by refcount and TTL; closing one on displacement would evict a channel the replacement may still share, since the pool key is (target, credentials, options) and an update that changes only tags or pid yields the same key. That rationale now lives on WorkerConnection, where channel lifetime is actually owned, and the sites that depend on it point there rather than restating it.

Select workers by uid in LoadBalancerLike.delegate

A selection names a worker; the proxy resolves it against the pool at dispatch time and connects through the entry it finds there. Staleness stops being something balancer authors must defend against and becomes something they cannot express.

The connection half of the yield was a round trip in any case — the balancer took a connection out of the pool and handed it back to the proxy, which owns the pool — and it was the half that went stale. Balancers that need a record or a connection to decide (match tags, pin a version, weigh a connection) still read them from context.workers; they simply no longer carry what they read back across the boundary. The success echo narrows with the yield: asend now sends back the uid.

Selecting a worker that has left the pool is no longer an error a balancer must avoid. The proxy finds no entry, requests the next candidate, and reports nothing back — a departed worker has not failed a dispatch.

Round-robin

The rotation snapshots uids once per wrap and indexes them in O(1). Because candidates are uids, a worker refreshed mid-cycle needs no special handling in the balancer at all. The cycle boundary is a uid and survives refreshes; it is reseeded only when that uid leaves the pool, since a departed uid can never recur and would otherwise never close the cycle.

Documentation

The load-balancer README's protocol excerpt declared the superseded (metadata, connection) signature and is replaced by a pointer to LoadBalancerLike, which owns the contract; the README now states the uid identity model in prose rather than only in its examples. The public docstrings on LoadBalancerLike, LoadBalancerContextView, LoadBalancerContext, and WorkerProxy.workers state the caller-visible contract and move mechanism into Implementation notes rubrics, so the guarantees a balancer author must honor are stated once, in the entity that owns them.

Breaking changes

This is a pre-1.0 surface, and both changes delete API rather than add it. A custom balancer migrates as follows:

# before
for metadata, connection in context.workers.items():
    sent = yield metadata, connection

# after
for uid in context.workers:
    sent = yield uid

update_worker no longer accepts upsert; call add_worker to admit a worker that may not be pooled yet.

Test cases

# Test Suite Given When Then Coverage Target
1 TestLoadBalancerContext A pooled worker A caller assigns through workers It should raise TypeError Read-only surface
2 TestLoadBalancerContext A workers view captured before any mutation A worker is admitted, a second refreshed, and a third evicted It should reflect all three mutations through the captured view Live-view contract
3 TestLoadBalancerContext A uid absent from the pool The worker is admitted It should hold exactly one entry for that uid add_worker admit
4 TestLoadBalancerContext A worker already in the pool It is re-admitted under the same uid with changed metadata It should replace the entry rather than duplicate it add_worker replace
5 TestLoadBalancerContext A worker evicted from the pool A changed record with that uid is admitted It should re-admit exactly one entry Re-admission after eviction
6 TestLoadBalancerContext Any non-empty subset of the non-uid fields mutated The worker is refreshed with the mutated record It should carry the new record and connection Refresh across the full field space
7 TestLoadBalancerContext A chain of successive record variants Each is applied in turn It should retain one entry carrying the last Repeated refresh
8 TestLoadBalancerContext A worker refreshed, so an earlier record is stale The stale record is written again It should regress the entry — writes are last-writer-wins Documented write semantics
9 TestLoadBalancerContext Three workers in a known order The middle one is refreshed It should preserve iteration order Rotation fairness under refresh
10 TestLoadBalancerContext A fresh-uid clone of a pooled worker's fields It is passed to update_worker It should change nothing Identity is uid, never field equality
11 TestLoadBalancerContext A worker evicted from the pool A changed record with that uid is refreshed It should not resurrect the worker Eviction retires a uid
12 TestLoadBalancerContext Any non-empty subset of the non-uid fields mutated The worker is evicted with the mutated record It should evict the entry Eviction matches by uid
13 TestLoadBalancerContext A uid absent from the pool The worker is evicted It should change nothing Eviction of an absent uid
14 TestLoadBalancerContext An empty context A worker is evicted It should change nothing Eviction on an empty pool
15 TestLoadBalancerContext A drawn sequence of admit, refresh, and evict operations over colliding uids Each is applied alongside a uid-keyed reference model It should equal the model after every operation Whole-surface model conformance (Hypothesis)
16 TestRoundRobinLoadBalancer An empty context The balancer is driven It should yield nothing Empty pool
17 TestRoundRobinLoadBalancer One pooled worker The balancer is driven It should yield that uid Single candidate
18 TestRoundRobinLoadBalancer A prior dispatch that succeeded A new call starts It should advance the cursor past the served worker Rotation across calls
19 TestRoundRobinLoadBalancer A candidate whose dispatch failed The generator is driven on It should advance to the next uid Failure advances the cursor
20 TestRoundRobinLoadBalancer Every worker failing The generator is driven to exhaustion It should end the cycle Cycle termination
21 TestRoundRobinLoadBalancer A worker evicted mid-cycle The generator is driven on It should skip the evicted uid Mutation during a cycle
22 TestRoundRobinLoadBalancer A candidate whose dispatch succeeded Success is reported via asend It should terminate the cycle Success ends the cycle
23 TestRoundRobinLoadBalancer Failures preceding a success The generator is driven It should offer workers in rotation order Ordering
24 TestRoundRobinLoadBalancer Successive delegate calls Each is driven It should rotate the starting worker Fairness across calls
25 TestRoundRobinLoadBalancer Two concurrent drivers Both are driven It should offer each distinct workers Concurrent delegation
26 TestRoundRobinLoadBalancer The checkpoint worker evicted The generator is driven It should terminate Boundary eviction
27 TestRoundRobinLoadBalancer A worker evicted after the snapshot The generator is driven It should skip that uid Snapshot versus live pool
28 TestRoundRobinLoadBalancer A sole worker refreshed after its yield The generator is driven It should exhaust the current cycle A refresh consumes the worker's one attempt
29 TestRoundRobinLoadBalancer A sole worker refreshed after its yield A new call starts It should offer the uid again, resolving to the fresh entry A refresh cannot drop a worker from the rotation
30 TestRoundRobinLoadBalancer A not-yet-reached worker refreshed mid-cycle The generator is driven to exhaustion It should visit each uid exactly once A refresh changes the record, not the cycle
31 TestRoundRobinLoadBalancer The checkpoint worker refreshed after its yield The generator is driven to termination It should terminate when the boundary uid recurs Boundary survives refresh
32 TestRoundRobinLoadBalancer 2-6 workers, a drawn warmup, and a refresh drawn at any position and step All dispatches fail transiently It should always terminate within bounds Termination sweep, including post-wrap refreshes (Hypothesis)
33 TestRoundRobinLoadBalancer A full rotation The cycle wraps It should snapshot the pool once per wrap O(1) cursor
34 TestRoundRobinLoadBalancer A retired context The balancer is released It should not pin the context Context lifetime
35 TestWorkerProxy lease=None Workers are discovered It should accept all of them Uncapped admission
36 TestWorkerProxy A pool at capacity A drop frees a slot It should accept the next worker Capacity accounting
37 TestWorkerProxy lease=1 and a cap-rejected worker A changed-record update arrives for it It should keep the worker out A refresh never admits
38 TestWorkerProxy A discovered worker worker-updated arrives with changed metadata It should refresh the pool entry Sentinel refresh (#292)
39 TestWorkerProxy A pool at capacity An admitted worker is updated It should refresh the entry without consuming a slot Admission is per uid
40 TestWorkerProxy A dropped worker A changed-record update arrives for it It should not resurrect the worker Post-drop guard
41 TestWorkerProxy lease=2 at capacity An admitted worker re-announces It should refresh its entry without consuming a slot Re-announcement at capacity
42 TestWorkerProxy A drawn sequence of added, updated, and dropped events over uids sharing an address The sentinel drains the stream It should mirror a uid-keyed reference model after every event Sentinel model conformance (Hypothesis)
43 TestWorkerProxy A worker refreshed to a new address A task is dispatched It should serve the task through the connection built from the update Dispatch follows the refresh
44 TestWorkerProxy A worker refreshed mid-dispatch, failing non-transiently Dispatch fails over It should evict the uid entirely and let the survivor serve uid-keyed eviction versus concurrent refresh
45 TestWorkerProxy The same refresh, failing transiently Dispatch runs It should preserve the refreshed entry Transient path preserves refreshes
46 TestWorkerProxy A balancer that selects a uid and only then lets a refresh land A task is dispatched It should dispatch through the refreshed connection, never the superseded one A selection cannot carry a stale address
47 TestWorkerProxy Two workers, the selected one leaving the pool before the proxy resolves it A task is dispatched It should skip the candidate with no failure reported against it Departed candidates are skipped, not failed
48 TestWorkerProxy A sole worker that leaves the pool after being selected A task is dispatched It should raise NoWorkersAvailable once the balancer exhausts Skipping cannot loop forever
49 TestWorkerProxy A legacy balancer evicting with a changed record dispatch() is called It should raise NoWorkersAvailable and leave the pool empty Deprecated mutable-context surface
50 TestWorkerProxy quorum=1 and an update-only discovery stream start() is awaited It should raise TimeoutError and leave no sentinel Updates cannot satisfy quorum
51 TestWorkerUpdatePropagation A live pool over real LocalDiscovery and a second live worker worker-updated republishes the uid at the new address It should follow the dispatch to the new worker End-to-end propagation
52 TestWorkerUpdatePropagation The refresh applied The original worker's process is stopped It should keep reaching the second worker The refresh replaced the connection
53 TestWorkerUpdatePropagation The refresh applied worker-dropped is published for the uid It should raise NoWorkersAvailable No ghost worker
54 TestWorkerUpdatePropagation lease=1 and a worker discovery registered but the pool never admitted worker-updated arrives for that worker It should raise NoWorkersAvailable — the update neither admits it nor grows the pool Removal of the upsert path, end to end

@conradbzura conradbzura self-assigned this Jul 13, 2026
@conradbzura conradbzura force-pushed the 292-fix-dropped-worker-updates branch 2 times, most recently from b37310f to 809b958 Compare July 14, 2026 02:41
@conradbzura conradbzura force-pushed the 292-fix-dropped-worker-updates branch 2 times, most recently from 5603b03 to 83d478a Compare July 14, 2026 17:06
WorkerMetadata hashes by uid alone while the generated dataclass
equality compares every field, so a pool keyed by the metadata record
could not address a worker whose record had changed. Every membership
test failed for exactly the events that mattered: the discovery
sentinel dropped worker-updated events carrying changed metadata, a
worker-dropped event whose record differed from the cached one missed
its entry, and a re-announcement inserted a duplicate that consumed a
second lease slot.

Key the pool by the worker's uid, which is what actually identifies a
worker: LoadBalancerContext maps uid to the worker's current record
and connection. Membership, lookup, and eviction all follow from the
key, so the mutators become single dict operations and a duplicate
entry is no longer possible to express. The lease cap counts distinct
uids, so a worker re-announcing itself refreshes its entry rather than
being turned away at capacity.

The upsert flag goes with the old key. Keying by uid gives add_worker
and update_worker distinct contracts — add admits or replaces, update
refreshes what is already admitted — so the boolean that told them
apart has nothing left to select.

Connections displaced by a refresh are deliberately left unclosed. A
connection is a lazy handle and the channel it names is owned by the
pooling layer, which reaps it by refcount and TTL; closing one here
would evict a channel the replacement may still share.

BREAKING CHANGE: LoadBalancerContextView.workers is now keyed by
uuid.UUID rather than WorkerMetadata, mapping each uid to a
(metadata, connection) pair. A custom balancer that iterates
context.workers.items() must iterate values() instead, and one that
tests membership with a metadata record must test with record.uid.
LoadBalancerContextLike.update_worker no longer accepts the upsert
keyword; call add_worker to admit a worker that may not be pooled yet.
A balancer yielded the (metadata, connection) pair it had read from
the pool, and the proxy dispatched through whatever pair it was handed.
But a balancer selects from a view it may have read before a discovery
refresh replaced a worker's entry: round-robin re-snapshots once per
wrap, and a custom balancer may snapshot once per call. The connection
it yielded could therefore name the worker's superseded address, which
left every balancer that does not check for it dispatching to a dead
endpoint — including the three this package documents as examples.

Yield the worker's uid instead. A selection names a worker; the proxy
resolves that name against the live pool at the moment it dispatches,
so a balancer cannot route a task to a superseded address however
stale the view it chose from. The staleness stops being something
balancer authors must defend against and becomes something they cannot
express.

The connection half of the yield was a round trip in any case: the
balancer took a connection out of the pool and handed it back to the
proxy, which owns the pool. It was also the half that went stale.
Balancers that need a record or a connection to decide — to match
tags, pin a version, weigh a connection — still read them from the
context; they simply no longer carry what they read back across the
boundary. The success echo narrows with the yield: asend now sends
back the uid.

Selecting a worker that has left the pool is no longer an error the
balancer must avoid. The proxy finds no entry, asks for the next
candidate, and reports nothing back, since a worker that has departed
has not failed a dispatch.

BREAKING CHANGE: LoadBalancerLike.delegate now yields uuid.UUID rather
than a (WorkerMetadata, WorkerConnection) pair, and receives a
uuid.UUID from asend. A custom balancer yields the uid it selected:
"yield metadata, connection" becomes "yield uid". Records and
connections remain available through context.workers, which maps each
uid to its current (metadata, connection) pair.
@conradbzura conradbzura force-pushed the 292-fix-dropped-worker-updates branch from 83d478a to 1ddd9fd Compare July 14, 2026 17:42
@conradbzura conradbzura marked this pull request as ready for review July 14, 2026 17:46
@conradbzura conradbzura merged commit 30425f6 into wool-labs:release Jul 14, 2026
11 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.

Discovery sentinel drops worker-updated events carrying changed metadata

1 participant