fix: one TOPIC per channel on netsplit heal (issue #1)#13
Open
vjt wants to merge 1 commit intoazzurra:masterfrom
Open
fix: one TOPIC per channel on netsplit heal (issue #1)#13vjt wants to merge 1 commit intoazzurra:masterfrom
vjt wants to merge 1 commit intoazzurra:masterfrom
Conversation
…rra#1) When a remote server finished bursting after a netsplit heal and the affected channels had CI_KEEPTOPIC + CI_TOPICLOCK set, ChanServ emitted two back-to-back TOPIC commands for each such channel: 1) chan_handle_SJOIN called restore_topic() the moment the channel was re-created (newChannel == TRUE + synched == TRUE). 2) The immediately-following TOPIC propagated from the same bursting server fell into chan_handle_TOPIC's server-source branch, which — gated only on synched == TRUE — overrode chan->topic with ci->last_topic and emitted a second ChanServ TOPIC with identical content. Services already track per-server burst state via SERVER_FLAG_BURSTING (set in servers_SERVER_create / server_handle_SERVER, cleared at m_gnotice's "has synched to network data." match). The flag just wasn't consulted by the TOPIC path. Fix: * channels.c / chan_handle_SJOIN: skip restore_topic() while the originating server is still BURSTING. * channels.c / chan_handle_TOPIC: in the server-source branch, skip both the TOPICLOCK override (and its send_cmd TOPIC) and the trailing record_topic() while the originating server is BURSTING. chan->topic is still updated via the normal store path so the in-memory state stays consistent; ci->last_topic is preserved so the reconciliation pass has something to compare against. * messages.c / m_gnotice: after clearing SERVER_FLAG_BURSTING, call synch_topics() to resolve any ci->last_topic vs chan->topic mismatch accumulated during the burst with a single ChanServ TOPIC per channel — the same logic already used for services' own initial sync. Result: one TOPIC per channel per netsplit heal, matching rfc1459's original three-point critique on the issue. Fixes azzurra#1
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
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
Closes #1 (open since 2011).
When a remote server finished bursting after a netsplit heal and the affected channels had
CI_KEEPTOPIC + CI_TOPICLOCKset, ChanServ emitted two back-to-backTOPICcommands per channel:chan_handle_SJOINcalledrestore_topic()the moment the channel was re-created (newChannel == TRUE && synched == TRUE).TOPICpropagated from the same bursting server fell intochan_handle_TOPIC's server-source branch. That branch was gated only onsynched == TRUE, so it fired, overrodechan->topicwithci->last_topic, and emitted a second ChanServTOPICwith identical content.Services already track per-server burst state via
SERVER_FLAG_BURSTING(set onSERVERentry, cleared inm_gnoticeat"has synched to network data."). The flag just wasn't consulted by the TOPIC path.Fix
channels.c/chan_handle_SJOIN— skiprestore_topic()while the originating server is stillBURSTING.channels.c/chan_handle_TOPIC— in the server-source branch, skip both theTOPICLOCKoverride (and itssend_cmd TOPIC) and the trailingrecord_topic()while the originating server isBURSTING.chan->topicis still updated via the normal store path so in-memory state stays consistent;ci->last_topicis preserved so the reconciliation pass has something to compare against.messages.c/m_gnotice— after clearingSERVER_FLAG_BURSTING, callsynch_topics()to resolve anyci->last_topicvschan->topicmismatch accumulated during the burst with a single ChanServTOPICper channel. This is the same logic already used for services' own initial sync (called inm_ping/m_burst).Result: one
TOPICper channel per netsplit heal.Addresses all three points rfc1459 raised on the original issue:
TOPICuntil burst is processed (per server).CI_TOPICLOCKis set and the server's topic differs from ChanServ's DB (viasynch_topics'sstr_equalscheck).GNOTICEpattern match for per-server burst-end; a dedicated CAPAB on bahamut would be cleaner but is out of scope for this fix.Test plan
-Wall -Wshadow -Wcast-align -Wsign-compare).TOPICemitted per channel on rejoin withKEEPTOPIC+TOPICLOCKset.synch_topics()is invoked fromm_burst/m_ping, unchanged).TOPICduring stable operation unaffected (user-source branch untouched;src_burstingis only written in the server-source branch).