fix(lib): free a silent parent's slot and pace the moves we report
A candidate that accepted the dial and then said nothing kept its slot for the session, and ten of those blocked every later batch; a link now closes after a minute of silence. A parent that flaps its level made us tell the server four messages per flap; moves are now told at most once a second, the latest one winning. A parent that reports level zero is its own root from then on, whatever it said before. The wait for a searcher's control connection runs from the last message queued for it, not the first. A successful login is pinned to reset the leaf.
This commit is contained in:
@@ -36,15 +36,22 @@ fn the_server_socket_keeps_itself_alive() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn parked_actor(port: u16) -> ServerActor {
|
fn parked_actor(port: u16) -> ServerActor {
|
||||||
let (tx, _rx) = std::sync::mpsc::channel();
|
parked_actor_with_client(port).0
|
||||||
ServerActor::new(
|
}
|
||||||
|
|
||||||
|
fn parked_actor_with_client(
|
||||||
|
port: u16,
|
||||||
|
) -> (ServerActor, std::sync::mpsc::Receiver<ClientOperation>) {
|
||||||
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
let actor = ServerActor::new(
|
||||||
PeerAddress::new("127.0.0.1".to_string(), port),
|
PeerAddress::new("127.0.0.1".to_string(), port),
|
||||||
tx,
|
tx,
|
||||||
0,
|
0,
|
||||||
false,
|
false,
|
||||||
0,
|
0,
|
||||||
0,
|
0,
|
||||||
)
|
);
|
||||||
|
(actor, rx)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -191,6 +198,22 @@ fn a_successful_login_marks_the_session_live_again() {
|
|||||||
assert_eq!(actor.session.loss(), None);
|
assert_eq!(actor.session.loss(), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A new session starts without a parent: the leaf drops whatever tree it
|
||||||
|
// hung from and announces its parentless stance from there.
|
||||||
|
#[test]
|
||||||
|
fn a_successful_login_resets_the_distributed_leaf() {
|
||||||
|
let (mut actor, client) = parked_actor_with_client(1);
|
||||||
|
|
||||||
|
actor.handle_login_status(true);
|
||||||
|
|
||||||
|
assert!(matches!(
|
||||||
|
client.try_recv(),
|
||||||
|
Ok(ClientOperation::ResetDistributed)
|
||||||
|
));
|
||||||
|
actor.handle_login_status(false);
|
||||||
|
assert!(client.try_recv().is_err(), "a failed login resets nothing");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn post_login_messages_carry_counts_and_conditional_wait_port() {
|
fn post_login_messages_carry_counts_and_conditional_wait_port() {
|
||||||
let messages = post_login_messages(true, 4321, 3, 7);
|
let messages = post_login_messages(true, 4321, 3, 7);
|
||||||
|
|||||||
@@ -20,6 +20,13 @@ use std::time::{Duration, Instant};
|
|||||||
const LINK_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
|
const LINK_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||||
/// How often an idle link looks at its cancel flag.
|
/// How often an idle link looks at its cancel flag.
|
||||||
const CANCEL_POLL: Duration = Duration::from_secs(1);
|
const CANCEL_POLL: Duration = Duration::from_secs(1);
|
||||||
|
/// A parent relays the network's whole search stream; one that says nothing
|
||||||
|
/// for this long is gone, and holding its slot would block every later
|
||||||
|
/// candidate.
|
||||||
|
const LINK_IDLE: Duration = Duration::from_mins(1);
|
||||||
|
/// Moves of our branch are told to the server at most this often; a parent
|
||||||
|
/// that flaps its level must not make us flood the server.
|
||||||
|
const MOVE_INTERVAL: Duration = Duration::from_secs(1);
|
||||||
/// A distributed frame is a search or a branch fact; anything bigger is not
|
/// A distributed frame is a search or a branch fact; anything bigger is not
|
||||||
/// one of ours.
|
/// one of ours.
|
||||||
const MAX_FRAME: usize = 64 * 1024;
|
const MAX_FRAME: usize = 64 * 1024;
|
||||||
@@ -82,6 +89,10 @@ pub struct Leaf {
|
|||||||
candidates: HashMap<String, Candidate>,
|
candidates: HashMap<String, Candidate>,
|
||||||
next_link: u64,
|
next_link: u64,
|
||||||
searches: (Instant, u32),
|
searches: (Instant, u32),
|
||||||
|
/// When we last told the server we moved, and whether a move since then
|
||||||
|
/// is still waiting to be told.
|
||||||
|
moved: Option<Instant>,
|
||||||
|
move_pending: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Leaf {
|
impl Leaf {
|
||||||
@@ -94,6 +105,8 @@ impl Leaf {
|
|||||||
candidates: HashMap::new(),
|
candidates: HashMap::new(),
|
||||||
next_link: 1,
|
next_link: 1,
|
||||||
searches: (Instant::now(), 0),
|
searches: (Instant::now(), 0),
|
||||||
|
moved: None,
|
||||||
|
move_pending: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -163,7 +176,7 @@ impl Leaf {
|
|||||||
}
|
}
|
||||||
let candidate = self.candidates.get_mut(username)?;
|
let candidate = self.candidates.get_mut(username)?;
|
||||||
candidate.level = u32::try_from(level).ok();
|
candidate.level = u32::try_from(level).ok();
|
||||||
if level == 0 && candidate.root.is_none() {
|
if level == 0 {
|
||||||
candidate.root = Some(username.to_string());
|
candidate.root = Some(username.to_string());
|
||||||
}
|
}
|
||||||
self.moved_with_parent(username)
|
self.moved_with_parent(username)
|
||||||
@@ -251,8 +264,24 @@ impl Leaf {
|
|||||||
if branch == self.branch {
|
if branch == self.branch {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
self.branch = branch.clone();
|
self.branch = branch;
|
||||||
Some(branch)
|
self.move_pending = true;
|
||||||
|
self.due_announcement(Instant::now())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The branch to tell the server about, if a move is waiting and the
|
||||||
|
/// last one was told at least [`MOVE_INTERVAL`] ago.
|
||||||
|
pub fn due_announcement(&mut self, now: Instant) -> Option<Branch> {
|
||||||
|
if !self.move_pending
|
||||||
|
|| self
|
||||||
|
.moved
|
||||||
|
.is_some_and(|at| now.duration_since(at) < MOVE_INTERVAL)
|
||||||
|
{
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
self.move_pending = false;
|
||||||
|
self.moved = Some(now);
|
||||||
|
Some(self.branch.clone())
|
||||||
}
|
}
|
||||||
|
|
||||||
fn cancel_candidates(&mut self) {
|
fn cancel_candidates(&mut self) {
|
||||||
@@ -328,18 +357,25 @@ fn link(
|
|||||||
.get_buffer(),
|
.get_buffer(),
|
||||||
)?;
|
)?;
|
||||||
let mut reader = MessageReader::new();
|
let mut reader = MessageReader::new();
|
||||||
|
let mut heard = Instant::now();
|
||||||
loop {
|
loop {
|
||||||
if dial.cancel.load(Ordering::Relaxed) {
|
if dial.cancel.load(Ordering::Relaxed) {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
match reader.read_from_socket(&mut stream) {
|
match reader.read_from_socket(&mut stream) {
|
||||||
Ok(()) => {}
|
Ok(()) => heard = Instant::now(),
|
||||||
Err(e)
|
Err(e)
|
||||||
if matches!(
|
if matches!(
|
||||||
e.kind(),
|
e.kind(),
|
||||||
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut
|
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut
|
||||||
) =>
|
) =>
|
||||||
{
|
{
|
||||||
|
if heard.elapsed() > LINK_IDLE {
|
||||||
|
return Err(io::Error::new(
|
||||||
|
io::ErrorKind::TimedOut,
|
||||||
|
"silent parent",
|
||||||
|
));
|
||||||
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
Err(e) => return Err(e),
|
Err(e) => return Err(e),
|
||||||
@@ -516,7 +552,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_parents_later_moves_move_us_too() {
|
fn the_parents_later_moves_move_us_too_but_are_told_once_a_second() {
|
||||||
let mut leaf = leaf();
|
let mut leaf = leaf();
|
||||||
let alice = dial(&mut leaf, &["alice"])["alice"].link;
|
let alice = dial(&mut leaf, &["alice"])["alice"].link;
|
||||||
leaf.branch_level("alice", alice, 0);
|
leaf.branch_level("alice", alice, 0);
|
||||||
@@ -528,9 +564,36 @@ mod tests {
|
|||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
leaf.branch_level("alice", alice, 2),
|
leaf.branch_level("alice", alice, 2),
|
||||||
|
None,
|
||||||
|
"a second move within the second waits"
|
||||||
|
);
|
||||||
|
assert_eq!(leaf.branch(), Branch::under("grand", 3), "but it took");
|
||||||
|
assert_eq!(leaf.due_announcement(Instant::now()), None);
|
||||||
|
assert_eq!(
|
||||||
|
leaf.due_announcement(Instant::now() + MOVE_INTERVAL),
|
||||||
Some(Branch::under("grand", 3))
|
Some(Branch::under("grand", 3))
|
||||||
);
|
);
|
||||||
assert_eq!(leaf.branch(), Branch::under("grand", 3));
|
assert_eq!(
|
||||||
|
leaf.due_announcement(Instant::now() + MOVE_INTERVAL),
|
||||||
|
None,
|
||||||
|
"told once"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_parent_that_becomes_a_root_is_the_root_from_then_on() {
|
||||||
|
let mut leaf = leaf();
|
||||||
|
let alice = dial(&mut leaf, &["alice"])["alice"].link;
|
||||||
|
leaf.branch_root("alice", alice, "grand");
|
||||||
|
leaf.branch_level("alice", alice, 5);
|
||||||
|
assert_eq!(
|
||||||
|
leaf.search_from("alice", alice),
|
||||||
|
Some(Branch::under("grand", 6))
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
leaf.branch_level("alice", alice, 0),
|
||||||
|
Some(Branch::under("alice", 1))
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -388,7 +388,8 @@ pub struct ClientContext {
|
|||||||
/// Peer listen addresses learned from GetPeerAddress responses.
|
/// Peer listen addresses learned from GetPeerAddress responses.
|
||||||
peer_addresses: HashMap<String, (String, u32)>,
|
peer_addresses: HashMap<String, (String, u32)>,
|
||||||
/// Peer messages waiting for a control connection to that peer, and
|
/// Peer messages waiting for a control connection to that peer, and
|
||||||
/// since when: a peer that never answers must not keep them forever.
|
/// when the last was queued: a peer that never answers must not keep
|
||||||
|
/// them forever.
|
||||||
pending_peer_messages:
|
pending_peer_messages:
|
||||||
HashMap<String, (Instant, Vec<crate::message::Message>)>,
|
HashMap<String, (Instant, Vec<crate::message::Message>)>,
|
||||||
/// Uploads we have offered, keyed by our transfer token.
|
/// Uploads we have offered, keyed by our transfer token.
|
||||||
@@ -741,11 +742,12 @@ impl ClientContext {
|
|||||||
username: &str,
|
username: &str,
|
||||||
message: crate::message::Message,
|
message: crate::message::Message,
|
||||||
) {
|
) {
|
||||||
self.pending_peer_messages
|
let entry = self
|
||||||
|
.pending_peer_messages
|
||||||
.entry(username.to_string())
|
.entry(username.to_string())
|
||||||
.or_insert_with(|| (Instant::now(), Vec::new()))
|
.or_insert_with(|| (Instant::now(), Vec::new()));
|
||||||
.1
|
entry.0 = Instant::now();
|
||||||
.push(message);
|
entry.1.push(message);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Remove and return the messages queued for `username`.
|
/// Remove and return the messages queued for `username`.
|
||||||
|
|||||||
@@ -26,6 +26,15 @@ impl Client {
|
|||||||
Self::sweep_stale_offers(&client_context);
|
Self::sweep_stale_offers(&client_context);
|
||||||
if let Ok(mut ctx) = client_context.write_safe() {
|
if let Ok(mut ctx) = client_context.write_safe() {
|
||||||
ctx.expire_pending_peer_messages(Instant::now());
|
ctx.expire_pending_peer_messages(Instant::now());
|
||||||
|
if let Some(branch) =
|
||||||
|
ctx.leaf.due_announcement(Instant::now())
|
||||||
|
{
|
||||||
|
super::distributed::announce(
|
||||||
|
ctx.server_sender.as_ref(),
|
||||||
|
&branch,
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
let operation = match next {
|
let operation = match next {
|
||||||
|
|||||||
Reference in New Issue
Block a user