RUST-1047 Ensure TopologyClosedEvent is the last SDAM event emitted #485
Conversation
| @@ -169,21 +173,6 @@ impl Topology { | |||
|
|
|||
| pub(crate) fn close(&self) { | |||
| self.common.is_alive.store(false, Ordering::SeqCst); | |||
| if let Some(ref handler) = self.common.options.sdam_event_handler { | |||
There was a problem hiding this comment.
This logic was moved to the drop of TopologyState, so it's guaranteed to come after any work the monitor does because the monitor holds onto a strong reference to the topology (Topology) while it's performing a heartbeat.
src/is_master.rs
Outdated
| /// Execute an isMaster command, emiting events if a reference to the topology and a handler are provided. | ||
| /// | ||
| /// A strong reference to the topology is used here to ensure it is still in scope and has not yet emitted a | ||
| /// `TopologyClosedEvent`. |
There was a problem hiding this comment.
We have to pass the Topology down through this similar to how we were doing it with WeakTopology before. It's a bit hairy right now, but I imagine this will all become a lot more straightforward and cleaned up once we do the topology refactor.
src/is_master.rs
Outdated
| cmap::{Command, Connection}, | ||
| error::Result, | ||
| event::sdam::{ | ||
| }, cmap::{Command, Connection}, error::Result, event::sdam::{ |
There was a problem hiding this comment.
Nit: did rustfmt change logic for use clauses? The grouping here looks odd.
There was a problem hiding this comment.
Good catch! Based on the way check-rustfmt.sh was written, files in src but not in a subdirectory (like is_master.rs) weren't getting linted for formatting. I've reformated this file and also updated check-rustfmt.sh to account for these files too.
RUST-1047
This PR fixes a race between server monitoring and topology closing that would occasionally result in heartbeat events being emitted after the final TopologyClosedEvent.