Sol/Fable Review of codebase.#3
Open
jeregrine wants to merge 12 commits into
Open
Conversation
Cluster membership was written to two public ETS tables by multiple shards and API callers. Interleaved add, remove, and purge operations could leave a one-sided row and keep a dead node in routing state. Route membership mutations through the Data owner and make purge scan the forward index so it also repairs existing orphaned rows.
jeregrine
changed the title
A local disconnect removed only locally owned rows, leaving remote registry and group entries behind. Deletes missed while disconnected could then survive the additive reconnect merge forever. Discard the complete local view of each departed cluster and release monitors for removed local entries. Validate that connect and disconnect receive binary cluster names so the nil default cluster can never be targeted by the full purge.
A replication batch already queued behind a local disconnect could repopulate the cluster after its entries were purged. Check named-cluster membership when buffered operations are applied so disconnected clusters cannot accept late registry or group writes.
Loser termination lived inside the default resolver, so custom resolvers converged the registry while leaving the losing process alive. Apply termination after either resolver chooses a winner and use the winner's metadata consistently in the exit reason. Document the behavior change in the changelog: resolvers that expect the losing process to keep running must now handle its termination.
Disconnect waited on each shard before sending to the next, so one timeout left later shards untouched after cluster membership was already removed. Start every local request first, then await replies with alias cleanup so all shards receive the purge even when one call times out.
Process-DOWN replication and dispatch still used ordinary remote sends, allowing a busy distribution channel to suspend a replica shard or application caller and to initiate unwanted connections. Route both paths through send_nosuspend with noconnect and the existing reconnect policy.
Configured one-argument extractors were documented but silently treated as identity functions, exposing unfiltered metadata on reads and lifecycle events. Recognize function extractors in both public read and replica event paths, and reject invalid extractor shapes at startup so a misconfigured callback cannot silently fall back to identity.
lookup rescued every ArgumentError, including failures raised by user extraction callbacks, and returned nil as if the key were absent. Limit recovery to the ETS access used for stopped instances so callback errors remain visible.
Zero and negative shard counts created descending ranges and supervisors that only failed later during routing. Apply the existing positive-integer check before configuration is stored or children are built.
TTL activity checks counted every matching registry or group row even though the sweeper needs only a boolean. Use limited ETS selects so each shard stops after the first match and later shards remain short-circuited.
Exact group keys are deterministically routed, but both count APIs scanned every shard. Compute the owning shard in the public API and query only that table while retaining all-shard scans for prefixes.
Public and internal docs still described DurableServer locking, built-in process groups, old logging behavior, and outdated replication paths. Make README and CLAUDE.md authoritative and drop the unused callbacks config entry. The _archdoc reference blocks in Replica, Data, and ClusterLease are kept in place.
jeregrine
force-pushed
the
js-review-fixes
branch
from
July 15, 2026 18:35
50910d1 to
e950f5d
Compare
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
1 participant
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.
Each commit is a single thing it found, ordered by priority so its best to click a commit read-through it yourself. We can always drop a commit if it doesn't fit with the intent of the group project.
https://gist.github.com/jeregrine/c50404bc9317ca08f6cebd0b32381c07 for original finding