Fire join notifications for new groups during initial sync#16
Merged
Conversation
When a new peer syncs its entries for the first time, groups that have no previous remote entries are unambiguous joins. Fire :join notifications for these so monitor subscribers are notified of all cluster members, not just those that register after the initial sync exchange completes. The existing sync_one_group path (re-sync of known groups) still skips notifications due to the multi-join ambiguity documented in the code.
twinn
force-pushed
the
fix/sync-join-notifications
branch
from
070fb0c to
bb97cf9
Compare
Previously, sync_groups and sync_one_group silently updated ETS
without firing monitor notifications. This meant subscribers were
not notified of membership changes that arrived via the initial
sync exchange or via re-sync after a netsplit.
The suppression was based on a misunderstanding about ambiguity in
diffing entries. Since each entry has a unique {pid, tag} pair,
joins and leaves can be unambiguously identified during sync.
Changes:
- sync_groups: fire :join for new groups from a peer
- sync_one_group: fire :join/:leave by diffing {pid, tag} sets
- Bump version to 0.4.1
twinn
force-pushed
the
fix/sync-join-notifications
branch
from
bb97cf9 to
104914f
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.
Summary
Fixes sync paths to fire monitor notifications for membership changes that arrive via the initial
:syncexchange or re-sync after reconnection.Previously,
sync_groupsandsync_one_groupsilently updated ETS without notifying monitor subscribers. This meant processes usingPgRegistry.monitor/2were not informed of members that registered on a peer before the local scope discovered that peer.The suppression was based on a misunderstanding about ambiguity in diffing entries during re-sync. Since each entry carries a unique
{pid, tag}pair, joins and leaves can be unambiguously identified.Changes
sync_groups: fire:joinnotifications for new groups from a peer (the{nil, _}branch — entirely new group, no diff ambiguity)sync_one_group: diff entries by{pid, tag}, fire:joinfor new entries and:leavefor removed entriesMotivation
Discovered while integrating PgRegistry into PartitionedEts for cluster membership with per-node metadata. When a new node joins and registers a table, existing nodes receive the entry via
:sync(not via the{:join, ...}broadcast, which races with the discover/sync exchange). Without this fix, existing nodes never receive the:joinmonitor event and never run handoff for the new node.Test plan
mix format --check-formattedmix credo --strict— no issues:joinfor entries arriving via initial sync:joinwhen a late-joining peer has pre-existing entriessync_one_groupfires:joinwhen a known peer adds entries to an existing group