From fcbceb07ae9bd4160a294ab5c982b6882eca6a7b Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Tue, 12 Mar 2024 12:43:21 +0100 Subject: [PATCH 1/2] Improve Endpoint and Locator doc --- commons/zenoh-protocol/src/core/endpoint.rs | 7 ++++++- commons/zenoh-protocol/src/core/locator.rs | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/commons/zenoh-protocol/src/core/endpoint.rs b/commons/zenoh-protocol/src/core/endpoint.rs index 5e921345e4..a8fcb3ae98 100644 --- a/commons/zenoh-protocol/src/core/endpoint.rs +++ b/commons/zenoh-protocol/src/core/endpoint.rs @@ -497,7 +497,12 @@ impl fmt::Debug for ConfigMut<'_> { } } -/// A `String` that respects the [`EndPoint`] canon form: `#`, such that `` is a valid [`Locator`] `` is of the form `=;...;=` where keys are alphabetically sorted. +/// A string that respects the [`EndPoint`] canon form: `[#]`. +/// +/// `` is a valid [`Locator`] and `` is of the form `=;...;=` where keys are alphabetically sorted. +/// `` is optional and can be provided to configure some aspectes for an [`EndPoint`], e.g. the interface to listen on or connect to. +/// +/// A full [`EndPoint`] string is hence in the form of `/
[?][#config]`. #[derive(Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] #[serde(into = "String")] #[serde(try_from = "String")] diff --git a/commons/zenoh-protocol/src/core/locator.rs b/commons/zenoh-protocol/src/core/locator.rs index 42379f2b65..50b909b12f 100644 --- a/commons/zenoh-protocol/src/core/locator.rs +++ b/commons/zenoh-protocol/src/core/locator.rs @@ -16,9 +16,9 @@ use alloc::{borrow::ToOwned, string::String}; use core::{convert::TryFrom, fmt, hash::Hash, str::FromStr}; use zenoh_result::{Error as ZError, ZResult}; -// Locator -/// A `String` that respects the [`Locator`] canon form: `/
[?]`, -/// such that `` is of the form `=;...;=` where keys are alphabetically sorted. +/// A string that respects the [`Locator`] canon form: `/
[?]`. +/// +/// `` is of the form `=;...;=` where keys are alphabetically sorted. #[derive(Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] #[serde(into = "String")] #[serde(try_from = "String")] From 41e25579f9f3c851f44c992946fd1d0c61fccfa9 Mon Sep 17 00:00:00 2001 From: OlivierHecart Date: Tue, 12 Mar 2024 13:42:18 +0100 Subject: [PATCH 2/2] Protocol changes: EntityId (into protocol_changes) (#774) * New Subscribers EntityId behavior for clients and peers * Improve routing logging * New Queryables EntityId behavior for clients and peers * Improve routing logging * Use proper QueryableId in Session and AdminSpace * Sessions use runtime Id generator to avoid collisions * AdminSpace use runtime Id generator to avoid collisions * Use proper ResponderId * Define EntityId type * Add source_eid to SourceInfo * Update source_info_stack_size test * Update source_info_stack_size test * Introduce EntityGlobalId type * Add id() function to Subscriber, Queryable and Publisher * Add Publication::with_source_info() function * Code format * Remove ref to PR #703 * Fix doctests * Add comments * Remove comments --- commons/zenoh-codec/src/network/declare.rs | 13 +- commons/zenoh-codec/src/network/mod.rs | 22 +- commons/zenoh-codec/src/zenoh/mod.rs | 26 +- commons/zenoh-protocol/src/core/mod.rs | 21 + commons/zenoh-protocol/src/core/wire_expr.rs | 4 + commons/zenoh-protocol/src/network/declare.rs | 17 +- commons/zenoh-protocol/src/network/mod.rs | 10 +- .../zenoh-protocol/src/network/response.rs | 2 +- commons/zenoh-protocol/src/zenoh/mod.rs | 10 +- zenoh/src/lib.rs | 2 +- zenoh/src/net/routing/dispatcher/face.rs | 6 +- zenoh/src/net/routing/dispatcher/pubsub.rs | 115 +++-- zenoh/src/net/routing/dispatcher/queries.rs | 103 ++-- zenoh/src/net/routing/dispatcher/resource.rs | 14 +- zenoh/src/net/routing/hat/client/mod.rs | 26 +- zenoh/src/net/routing/hat/client/pubsub.rs | 107 ++-- zenoh/src/net/routing/hat/client/queries.rs | 113 +++-- .../src/net/routing/hat/linkstate_peer/mod.rs | 30 +- .../net/routing/hat/linkstate_peer/pubsub.rs | 136 ++--- .../net/routing/hat/linkstate_peer/queries.rs | 142 +++--- zenoh/src/net/routing/hat/mod.rs | 17 +- zenoh/src/net/routing/hat/p2p_peer/mod.rs | 29 +- zenoh/src/net/routing/hat/p2p_peer/pubsub.rs | 106 ++-- zenoh/src/net/routing/hat/p2p_peer/queries.rs | 113 +++-- zenoh/src/net/routing/hat/router/mod.rs | 31 +- zenoh/src/net/routing/hat/router/pubsub.rs | 213 ++++---- zenoh/src/net/routing/hat/router/queries.rs | 220 +++++---- zenoh/src/net/runtime/adminspace.rs | 8 +- zenoh/src/net/runtime/mod.rs | 8 + zenoh/src/net/tests/tables.rs | 88 +++- zenoh/src/prelude.rs | 5 + zenoh/src/publication.rs | 83 +++- zenoh/src/queryable.rs | 73 +-- zenoh/src/sample.rs | 20 +- zenoh/src/session.rs | 467 +++++++----------- zenoh/src/subscriber.rs | 26 + 36 files changed, 1349 insertions(+), 1077 deletions(-) diff --git a/commons/zenoh-codec/src/network/declare.rs b/commons/zenoh-codec/src/network/declare.rs index 6df25a8d2a..bcc55ed62b 100644 --- a/commons/zenoh-codec/src/network/declare.rs +++ b/commons/zenoh-codec/src/network/declare.rs @@ -441,14 +441,19 @@ where let subscriber::UndeclareSubscriber { id, ext_wire_expr } = x; // Header - let header = declare::id::U_SUBSCRIBER | subscriber::flag::Z; + let mut header = declare::id::U_SUBSCRIBER; + if !ext_wire_expr.is_null() { + header |= subscriber::flag::Z; + } self.write(&mut *writer, header)?; // Body self.write(&mut *writer, id)?; // Extension - self.write(&mut *writer, (ext_wire_expr, false))?; + if !ext_wire_expr.is_null() { + self.write(&mut *writer, (ext_wire_expr, false))?; + } Ok(()) } @@ -483,7 +488,6 @@ where let id: subscriber::SubscriberId = self.codec.read(&mut *reader)?; // Extensions - // WARNING: this is a temporary and mandatory extension used for undeclarations let mut ext_wire_expr = common::ext::WireExprType::null(); let mut has_ext = imsg::has_flag(self.header, subscriber::flag::Z); @@ -665,7 +669,6 @@ where let id: queryable::QueryableId = self.codec.read(&mut *reader)?; // Extensions - // WARNING: this is a temporary and mandatory extension used for undeclarations let mut ext_wire_expr = common::ext::WireExprType::null(); let mut has_ext = imsg::has_flag(self.header, queryable::flag::Z); @@ -813,7 +816,6 @@ where let id: token::TokenId = self.codec.read(&mut *reader)?; // Extensions - // WARNING: this is a temporary and mandatory extension used for undeclarations let mut ext_wire_expr = common::ext::WireExprType::null(); let mut has_ext = imsg::has_flag(self.header, interest::flag::Z); @@ -1032,7 +1034,6 @@ where let id: interest::InterestId = self.codec.read(&mut *reader)?; // Extensions - // WARNING: this is a temporary and mandatory extension used for undeclarations let mut ext_wire_expr = common::ext::WireExprType::null(); let mut has_ext = imsg::has_flag(self.header, interest::flag::Z); diff --git a/commons/zenoh-codec/src/network/mod.rs b/commons/zenoh-codec/src/network/mod.rs index dade13d362..3a227cd42a 100644 --- a/commons/zenoh-codec/src/network/mod.rs +++ b/commons/zenoh-codec/src/network/mod.rs @@ -26,8 +26,8 @@ use zenoh_buffers::{ }; use zenoh_protocol::{ common::{imsg, ZExtZ64, ZExtZBufHeader}, - core::{Reliability, ZenohId}, - network::{ext::EntityIdType, *}, + core::{EntityId, Reliability, ZenohId}, + network::{ext::EntityGlobalIdType, *}, }; // NetworkMessage @@ -218,21 +218,21 @@ where } // Extension: EntityId -impl LCodec<&ext::EntityIdType<{ ID }>> for Zenoh080 { - fn w_len(self, x: &ext::EntityIdType<{ ID }>) -> usize { - let EntityIdType { zid, eid } = x; +impl LCodec<&ext::EntityGlobalIdType<{ ID }>> for Zenoh080 { + fn w_len(self, x: &ext::EntityGlobalIdType<{ ID }>) -> usize { + let EntityGlobalIdType { zid, eid } = x; 1 + self.w_len(zid) + self.w_len(*eid) } } -impl WCodec<(&ext::EntityIdType<{ ID }>, bool), &mut W> for Zenoh080 +impl WCodec<(&ext::EntityGlobalIdType<{ ID }>, bool), &mut W> for Zenoh080 where W: Writer, { type Output = Result<(), DidntWrite>; - fn write(self, writer: &mut W, x: (&ext::EntityIdType<{ ID }>, bool)) -> Self::Output { + fn write(self, writer: &mut W, x: (&ext::EntityGlobalIdType<{ ID }>, bool)) -> Self::Output { let (x, more) = x; let header: ZExtZBufHeader<{ ID }> = ZExtZBufHeader::new(self.w_len(x)); self.write(&mut *writer, (&header, more))?; @@ -248,13 +248,13 @@ where } } -impl RCodec<(ext::EntityIdType<{ ID }>, bool), &mut R> for Zenoh080Header +impl RCodec<(ext::EntityGlobalIdType<{ ID }>, bool), &mut R> for Zenoh080Header where R: Reader, { type Error = DidntRead; - fn read(self, reader: &mut R) -> Result<(ext::EntityIdType<{ ID }>, bool), Self::Error> { + fn read(self, reader: &mut R) -> Result<(ext::EntityGlobalIdType<{ ID }>, bool), Self::Error> { let (_, more): (ZExtZBufHeader<{ ID }>, bool) = self.read(&mut *reader)?; let flags: u8 = self.codec.read(&mut *reader)?; @@ -263,8 +263,8 @@ where let lodec = Zenoh080Length::new(length); let zid: ZenohId = lodec.read(&mut *reader)?; - let eid: u32 = self.codec.read(&mut *reader)?; + let eid: EntityId = self.codec.read(&mut *reader)?; - Ok((ext::EntityIdType { zid, eid }, more)) + Ok((ext::EntityGlobalIdType { zid, eid }, more)) } } diff --git a/commons/zenoh-codec/src/zenoh/mod.rs b/commons/zenoh-codec/src/zenoh/mod.rs index fdff09be94..0d7146dc90 100644 --- a/commons/zenoh-codec/src/zenoh/mod.rs +++ b/commons/zenoh-codec/src/zenoh/mod.rs @@ -32,7 +32,7 @@ use zenoh_buffers::{ use zenoh_protocol::common::{iext, ZExtUnit}; use zenoh_protocol::{ common::{imsg, ZExtZBufHeader}, - core::{Encoding, ZenohId}, + core::{Encoding, EntityGlobalId, EntityId, ZenohId}, zenoh::{ext, id, PushBody, RequestBody, ResponseBody}, }; @@ -150,9 +150,9 @@ where // Extension: SourceInfo impl LCodec<&ext::SourceInfoType<{ ID }>> for Zenoh080 { fn w_len(self, x: &ext::SourceInfoType<{ ID }>) -> usize { - let ext::SourceInfoType { zid, eid, sn } = x; + let ext::SourceInfoType { id, sn } = x; - 1 + self.w_len(zid) + self.w_len(*eid) + self.w_len(*sn) + 1 + self.w_len(&id.zid) + self.w_len(id.eid) + self.w_len(*sn) } } @@ -164,18 +164,18 @@ where fn write(self, writer: &mut W, x: (&ext::SourceInfoType<{ ID }>, bool)) -> Self::Output { let (x, more) = x; - let ext::SourceInfoType { zid, eid, sn } = x; + let ext::SourceInfoType { id, sn } = x; let header: ZExtZBufHeader<{ ID }> = ZExtZBufHeader::new(self.w_len(x)); self.write(&mut *writer, (&header, more))?; - let flags: u8 = (zid.size() as u8 - 1) << 4; + let flags: u8 = (id.zid.size() as u8 - 1) << 4; self.write(&mut *writer, flags)?; - let lodec = Zenoh080Length::new(zid.size()); - lodec.write(&mut *writer, zid)?; + let lodec = Zenoh080Length::new(id.zid.size()); + lodec.write(&mut *writer, &id.zid)?; - self.write(&mut *writer, eid)?; + self.write(&mut *writer, id.eid)?; self.write(&mut *writer, sn)?; Ok(()) } @@ -196,10 +196,16 @@ where let lodec = Zenoh080Length::new(length); let zid: ZenohId = lodec.read(&mut *reader)?; - let eid: u32 = self.codec.read(&mut *reader)?; + let eid: EntityId = self.codec.read(&mut *reader)?; let sn: u32 = self.codec.read(&mut *reader)?; - Ok((ext::SourceInfoType { zid, eid, sn }, more)) + Ok(( + ext::SourceInfoType { + id: EntityGlobalId { zid, eid }, + sn, + }, + more, + )) } } diff --git a/commons/zenoh-protocol/src/core/mod.rs b/commons/zenoh-protocol/src/core/mod.rs index 82658db2fd..20fcf85dd9 100644 --- a/commons/zenoh-protocol/src/core/mod.rs +++ b/commons/zenoh-protocol/src/core/mod.rs @@ -261,6 +261,27 @@ impl<'de> serde::Deserialize<'de> for ZenohId { } } +/// The unique id of a zenoh entity inside it's parent [`Session`]. +pub type EntityId = u32; + +/// The global unique id of a zenoh entity. +#[derive(Debug, Default, Clone, Eq, Hash, PartialEq)] +pub struct EntityGlobalId { + pub zid: ZenohId, + pub eid: EntityId, +} + +impl EntityGlobalId { + #[cfg(feature = "test")] + pub fn rand() -> Self { + use rand::Rng; + Self { + zid: ZenohId::rand(), + eid: rand::thread_rng().gen(), + } + } +} + #[repr(u8)] #[derive(Debug, Default, Copy, Clone, Eq, Hash, PartialEq)] pub enum Priority { diff --git a/commons/zenoh-protocol/src/core/wire_expr.rs b/commons/zenoh-protocol/src/core/wire_expr.rs index 6d9623d6ca..a66b1aa212 100644 --- a/commons/zenoh-protocol/src/core/wire_expr.rs +++ b/commons/zenoh-protocol/src/core/wire_expr.rs @@ -71,6 +71,10 @@ impl<'a> WireExpr<'a> { } } + pub fn is_empty(&self) -> bool { + self.scope == 0 && self.suffix.as_ref().is_empty() + } + pub fn as_str(&'a self) -> &'a str { if self.scope == 0 { self.suffix.as_ref() diff --git a/commons/zenoh-protocol/src/network/declare.rs b/commons/zenoh-protocol/src/network/declare.rs index 8164d9440d..2dd8de4ef8 100644 --- a/commons/zenoh-protocol/src/network/declare.rs +++ b/commons/zenoh-protocol/src/network/declare.rs @@ -177,7 +177,6 @@ pub mod common { pub mod ext { use super::*; - // WARNING: this is a temporary and mandatory extension used for undeclarations pub type WireExprExt = zextzbuf!(0x0f, true); #[derive(Debug, Clone, PartialEq, Eq)] pub struct WireExprType { @@ -195,6 +194,10 @@ pub mod common { } } + pub fn is_null(&self) -> bool { + self.wire_expr.is_empty() + } + #[cfg(feature = "test")] pub fn rand() -> Self { Self { @@ -286,9 +289,11 @@ pub mod keyexpr { } pub mod subscriber { + use crate::core::EntityId; + use super::*; - pub type SubscriberId = u32; + pub type SubscriberId = EntityId; pub mod flag { pub const N: u8 = 1 << 5; // 0x20 Named if N==1 then the key expr has name/suffix @@ -441,7 +446,6 @@ pub mod subscriber { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UndeclareSubscriber { pub id: SubscriberId, - // WARNING: this is a temporary and mandatory extension used for undeclarations pub ext_wire_expr: common::ext::WireExprType, } @@ -460,9 +464,11 @@ pub mod subscriber { } pub mod queryable { + use crate::core::EntityId; + use super::*; - pub type QueryableId = u32; + pub type QueryableId = EntityId; pub mod flag { pub const N: u8 = 1 << 5; // 0x20 Named if N==1 then the key expr has name/suffix @@ -597,7 +603,6 @@ pub mod queryable { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UndeclareQueryable { pub id: QueryableId, - // WARNING: this is a temporary and mandatory extension used for undeclarations pub ext_wire_expr: common::ext::WireExprType, } @@ -683,7 +688,6 @@ pub mod token { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UndeclareToken { pub id: TokenId, - // WARNING: this is a temporary and mandatory extension used for undeclarations pub ext_wire_expr: common::ext::WireExprType, } @@ -1097,7 +1101,6 @@ pub mod interest { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UndeclareInterest { pub id: InterestId, - // WARNING: this is a temporary and mandatory extension used for undeclarations pub ext_wire_expr: common::ext::WireExprType, } diff --git a/commons/zenoh-protocol/src/network/mod.rs b/commons/zenoh-protocol/src/network/mod.rs index bb76cb8946..6af7fef243 100644 --- a/commons/zenoh-protocol/src/network/mod.rs +++ b/commons/zenoh-protocol/src/network/mod.rs @@ -200,7 +200,7 @@ impl From for NetworkMessage { pub mod ext { use crate::{ common::{imsg, ZExtZ64}, - core::{CongestionControl, Priority, ZenohId}, + core::{CongestionControl, EntityId, Priority, ZenohId}, }; use core::fmt; @@ -407,19 +407,19 @@ pub mod ext { /// % eid % /// +---------------+ #[derive(Debug, Clone, PartialEq, Eq)] - pub struct EntityIdType { + pub struct EntityGlobalIdType { pub zid: ZenohId, - pub eid: u32, + pub eid: EntityId, } - impl EntityIdType<{ ID }> { + impl EntityGlobalIdType<{ ID }> { #[cfg(feature = "test")] pub fn rand() -> Self { use rand::Rng; let mut rng = rand::thread_rng(); let zid = ZenohId::rand(); - let eid: u32 = rng.gen(); + let eid: EntityId = rng.gen(); Self { zid, eid } } } diff --git a/commons/zenoh-protocol/src/network/response.rs b/commons/zenoh-protocol/src/network/response.rs index 9ef2c26a10..6f0925429b 100644 --- a/commons/zenoh-protocol/src/network/response.rs +++ b/commons/zenoh-protocol/src/network/response.rs @@ -67,7 +67,7 @@ pub mod ext { pub type TimestampType = crate::network::ext::TimestampType<{ Timestamp::ID }>; pub type ResponderId = zextzbuf!(0x3, false); - pub type ResponderIdType = crate::network::ext::EntityIdType<{ ResponderId::ID }>; + pub type ResponderIdType = crate::network::ext::EntityGlobalIdType<{ ResponderId::ID }>; } impl Response { diff --git a/commons/zenoh-protocol/src/zenoh/mod.rs b/commons/zenoh-protocol/src/zenoh/mod.rs index 1284116888..3e5d573c43 100644 --- a/commons/zenoh-protocol/src/zenoh/mod.rs +++ b/commons/zenoh-protocol/src/zenoh/mod.rs @@ -158,7 +158,7 @@ impl From for ResponseBody { pub mod ext { use zenoh_buffers::ZBuf; - use crate::core::{Encoding, ZenohId}; + use crate::core::{Encoding, EntityGlobalId}; /// 7 6 5 4 3 2 1 0 /// +-+-+-+-+-+-+-+-+ @@ -172,8 +172,7 @@ pub mod ext { /// +---------------+ #[derive(Debug, Clone, PartialEq, Eq)] pub struct SourceInfoType { - pub zid: ZenohId, - pub eid: u32, + pub id: EntityGlobalId, pub sn: u32, } @@ -183,10 +182,9 @@ pub mod ext { use rand::Rng; let mut rng = rand::thread_rng(); - let zid = ZenohId::rand(); - let eid: u32 = rng.gen(); + let id = EntityGlobalId::rand(); let sn: u32 = rng.gen(); - Self { zid, eid, sn } + Self { id, sn } } } diff --git a/zenoh/src/lib.rs b/zenoh/src/lib.rs index bae81d3a54..eb1ba1bcd1 100644 --- a/zenoh/src/lib.rs +++ b/zenoh/src/lib.rs @@ -79,7 +79,7 @@ extern crate zenoh_core; #[macro_use] extern crate zenoh_result; -pub(crate) type Id = usize; +pub(crate) type Id = u32; use git_version::git_version; use handlers::DefaultHandler; diff --git a/zenoh/src/net/routing/dispatcher/face.rs b/zenoh/src/net/routing/dispatcher/face.rs index 6ef5c063d0..79c9da9127 100644 --- a/zenoh/src/net/routing/dispatcher/face.rs +++ b/zenoh/src/net/routing/dispatcher/face.rs @@ -171,6 +171,7 @@ impl Primitives for Face { ctrl_lock.as_ref(), &self.tables, &mut self.state.clone(), + m.id, &m.wire_expr, &m.ext_info, msg.ext_nodeid.node_id, @@ -181,6 +182,7 @@ impl Primitives for Face { ctrl_lock.as_ref(), &self.tables, &mut self.state.clone(), + m.id, &m.ext_wire_expr.wire_expr, msg.ext_nodeid.node_id, ); @@ -190,6 +192,7 @@ impl Primitives for Face { ctrl_lock.as_ref(), &self.tables, &mut self.state.clone(), + m.id, &m.wire_expr, &m.ext_info, msg.ext_nodeid.node_id, @@ -200,6 +203,7 @@ impl Primitives for Face { ctrl_lock.as_ref(), &self.tables, &mut self.state.clone(), + m.id, &m.ext_wire_expr.wire_expr, msg.ext_nodeid.node_id, ); @@ -244,7 +248,7 @@ impl Primitives for Face { pull_data(&self.tables.tables, &self.state.clone(), msg.wire_expr); } _ => { - log::error!("Unsupported request"); + log::error!("{} Unsupported request!", self); } } } diff --git a/zenoh/src/net/routing/dispatcher/pubsub.rs b/zenoh/src/net/routing/dispatcher/pubsub.rs index d6497a80b3..c0d1bb4a34 100644 --- a/zenoh/src/net/routing/dispatcher/pubsub.rs +++ b/zenoh/src/net/routing/dispatcher/pubsub.rs @@ -22,7 +22,7 @@ use std::sync::RwLock; use zenoh_core::zread; use zenoh_protocol::core::key_expr::{keyexpr, OwnedKeyExpr}; use zenoh_protocol::network::declare::subscriber::ext::SubscriberInfo; -use zenoh_protocol::network::declare::Mode; +use zenoh_protocol::network::declare::{Mode, SubscriberId}; use zenoh_protocol::{ core::{WhatAmI, WireExpr}, network::{declare::ext, Push}, @@ -34,17 +34,24 @@ pub(crate) fn declare_subscription( hat_code: &(dyn HatTrait + Send + Sync), tables: &TablesLock, face: &mut Arc, + id: SubscriberId, expr: &WireExpr, sub_info: &SubscriberInfo, node_id: NodeId, ) { - log::debug!("Declare subscription {}", face); let rtables = zread!(tables.tables); match rtables .get_mapping(face, &expr.scope, expr.mapping) .cloned() { Some(mut prefix) => { + log::debug!( + "{} Declare subscriber {} ({}{})", + face, + id, + prefix.expr(), + expr.suffix + ); let res = Resource::get_resource(&prefix, &expr.suffix); let (mut res, mut wtables) = if res.as_ref().map(|r| r.context.is_some()).unwrap_or(false) { @@ -66,7 +73,7 @@ pub(crate) fn declare_subscription( (res, wtables) }; - hat_code.declare_subscription(&mut wtables, face, &mut res, sub_info, node_id); + hat_code.declare_subscription(&mut wtables, face, id, &mut res, sub_info, node_id); disable_matches_data_routes(&mut wtables, &mut res); drop(wtables); @@ -86,7 +93,12 @@ pub(crate) fn declare_subscription( } drop(wtables); } - None => log::error!("Declare subscription for unknown scope {}!", expr.scope), + None => log::error!( + "{} Declare subscriber {} for unknown scope {}!", + face, + id, + expr.scope + ), } } @@ -94,41 +106,60 @@ pub(crate) fn undeclare_subscription( hat_code: &(dyn HatTrait + Send + Sync), tables: &TablesLock, face: &mut Arc, + id: SubscriberId, expr: &WireExpr, node_id: NodeId, ) { - log::debug!("Undeclare subscription {}", face); - let rtables = zread!(tables.tables); - match rtables.get_mapping(face, &expr.scope, expr.mapping) { - Some(prefix) => match Resource::get_resource(prefix, expr.suffix.as_ref()) { - Some(mut res) => { - drop(rtables); - let mut wtables = zwrite!(tables.tables); - - hat_code.undeclare_subscription(&mut wtables, face, &mut res, node_id); - - disable_matches_data_routes(&mut wtables, &mut res); - drop(wtables); - - let rtables = zread!(tables.tables); - let matches_data_routes = compute_matches_data_routes(&rtables, &res); - drop(rtables); - - let wtables = zwrite!(tables.tables); - for (mut res, data_routes, matching_pulls) in matches_data_routes { - get_mut_unchecked(&mut res) - .context_mut() - .update_data_routes(data_routes); - get_mut_unchecked(&mut res) - .context_mut() - .update_matching_pulls(matching_pulls); + let res = if expr.is_empty() { + None + } else { + let rtables = zread!(tables.tables); + match rtables.get_mapping(face, &expr.scope, expr.mapping) { + Some(prefix) => match Resource::get_resource(prefix, expr.suffix.as_ref()) { + Some(res) => Some(res), + None => { + log::error!( + "{} Undeclare unknown subscriber {}{}!", + face, + prefix.expr(), + expr.suffix + ); + return; } - Resource::clean(&mut res); - drop(wtables); + }, + None => { + log::error!( + "{} Undeclare subscriber with unknown scope {}", + face, + expr.scope + ); + return; } - None => log::error!("Undeclare unknown subscription!"), - }, - None => log::error!("Undeclare subscription with unknown scope!"), + } + }; + let mut wtables = zwrite!(tables.tables); + if let Some(mut res) = hat_code.undeclare_subscription(&mut wtables, face, id, res, node_id) { + log::debug!("{} Undeclare subscriber {} ({})", face, id, res.expr()); + disable_matches_data_routes(&mut wtables, &mut res); + drop(wtables); + + let rtables = zread!(tables.tables); + let matches_data_routes = compute_matches_data_routes(&rtables, &res); + drop(rtables); + + let wtables = zwrite!(tables.tables); + for (mut res, data_routes, matching_pulls) in matches_data_routes { + get_mut_unchecked(&mut res) + .context_mut() + .update_data_routes(data_routes); + get_mut_unchecked(&mut res) + .context_mut() + .update_matching_pulls(matching_pulls); + } + Resource::clean(&mut res); + drop(wtables); + } else { + log::error!("{} Undeclare unknown subscriber {}", face, id); } } @@ -445,7 +476,8 @@ pub fn full_reentrant_route_data( match tables.get_mapping(face, &expr.scope, expr.mapping).cloned() { Some(prefix) => { log::trace!( - "Route data for res {}{}", + "{} Route data for res {}{}", + face, prefix.expr(), expr.suffix.as_ref() ); @@ -561,7 +593,7 @@ pub fn full_reentrant_route_data( } } None => { - log::error!("Route data with unknown scope {}!", expr.scope); + log::error!("{} Route data with unknown scope {}!", face, expr.scope); } } } @@ -602,14 +634,16 @@ pub fn pull_data(tables_ref: &RwLock, face: &Arc, expr: WireE } None => { log::error!( - "Pull data for unknown subscription {} (no info)!", + "{} Pull data for unknown subscriber {} (no info)!", + face, prefix.expr() + expr.suffix.as_ref() ); } }, None => { log::error!( - "Pull data for unknown subscription {} (no context)!", + "{} Pull data for unknown subscriber {} (no context)!", + face, prefix.expr() + expr.suffix.as_ref() ); } @@ -617,13 +651,14 @@ pub fn pull_data(tables_ref: &RwLock, face: &Arc, expr: WireE } None => { log::error!( - "Pull data for unknown subscription {} (no resource)!", + "{} Pull data for unknown subscriber {} (no resource)!", + face, prefix.expr() + expr.suffix.as_ref() ); } }, None => { - log::error!("Pull data with unknown scope {}!", expr.scope); + log::error!("{} Pull data with unknown scope {}!", face, expr.scope); } }; } diff --git a/zenoh/src/net/routing/dispatcher/queries.rs b/zenoh/src/net/routing/dispatcher/queries.rs index b0f7f7f7ef..287621151a 100644 --- a/zenoh/src/net/routing/dispatcher/queries.rs +++ b/zenoh/src/net/routing/dispatcher/queries.rs @@ -21,16 +21,14 @@ use async_trait::async_trait; use std::collections::HashMap; use std::sync::{Arc, Weak}; use zenoh_config::WhatAmI; -use zenoh_protocol::zenoh::reply::ReplyBody; -use zenoh_protocol::zenoh::Put; use zenoh_protocol::{ core::{key_expr::keyexpr, Encoding, WireExpr}, network::{ - declare::{ext, queryable::ext::QueryableInfo}, + declare::{ext, queryable::ext::QueryableInfo, QueryableId}, request::{ext::TargetType, Request, RequestId}, response::{self, ext::ResponderIdType, Response, ResponseFinal}, }, - zenoh::{query::Consolidation, Reply, RequestBody, ResponseBody}, + zenoh::{query::Consolidation, reply::ReplyBody, Put, Reply, RequestBody, ResponseBody}, }; use zenoh_sync::get_mut_unchecked; use zenoh_util::Timed; @@ -44,17 +42,24 @@ pub(crate) fn declare_queryable( hat_code: &(dyn HatTrait + Send + Sync), tables: &TablesLock, face: &mut Arc, + id: QueryableId, expr: &WireExpr, qabl_info: &QueryableInfo, node_id: NodeId, ) { - log::debug!("Register queryable {}", face); let rtables = zread!(tables.tables); match rtables .get_mapping(face, &expr.scope, expr.mapping) .cloned() { Some(mut prefix) => { + log::debug!( + "{} Declare queryable {} ({}{})", + face, + id, + prefix.expr(), + expr.suffix + ); let res = Resource::get_resource(&prefix, &expr.suffix); let (mut res, mut wtables) = if res.as_ref().map(|r| r.context.is_some()).unwrap_or(false) { @@ -76,7 +81,7 @@ pub(crate) fn declare_queryable( (res, wtables) }; - hat_code.declare_queryable(&mut wtables, face, &mut res, qabl_info, node_id); + hat_code.declare_queryable(&mut wtables, face, id, &mut res, qabl_info, node_id); disable_matches_query_routes(&mut wtables, &mut res); drop(wtables); @@ -93,7 +98,12 @@ pub(crate) fn declare_queryable( } drop(wtables); } - None => log::error!("Declare queryable for unknown scope {}!", expr.scope), + None => log::error!( + "{} Declare queryable {} for unknown scope {}!", + face, + id, + expr.scope + ), } } @@ -101,37 +111,57 @@ pub(crate) fn undeclare_queryable( hat_code: &(dyn HatTrait + Send + Sync), tables: &TablesLock, face: &mut Arc, + id: QueryableId, expr: &WireExpr, node_id: NodeId, ) { - let rtables = zread!(tables.tables); - match rtables.get_mapping(face, &expr.scope, expr.mapping) { - Some(prefix) => match Resource::get_resource(prefix, expr.suffix.as_ref()) { - Some(mut res) => { - drop(rtables); - let mut wtables = zwrite!(tables.tables); - - hat_code.undeclare_queryable(&mut wtables, face, &mut res, node_id); - - disable_matches_query_routes(&mut wtables, &mut res); - drop(wtables); - - let rtables = zread!(tables.tables); - let matches_query_routes = compute_matches_query_routes(&rtables, &res); - drop(rtables); - - let wtables = zwrite!(tables.tables); - for (mut res, query_routes) in matches_query_routes { - get_mut_unchecked(&mut res) - .context_mut() - .update_query_routes(query_routes); + let res = if expr.is_empty() { + None + } else { + let rtables = zread!(tables.tables); + match rtables.get_mapping(face, &expr.scope, expr.mapping) { + Some(prefix) => match Resource::get_resource(prefix, expr.suffix.as_ref()) { + Some(res) => Some(res), + None => { + log::error!( + "{} Undeclare unknown queryable {}{}!", + face, + prefix.expr(), + expr.suffix + ); + return; } - Resource::clean(&mut res); - drop(wtables); + }, + None => { + log::error!( + "{} Undeclare queryable with unknown scope {}", + face, + expr.scope + ); + return; } - None => log::error!("Undeclare unknown queryable!"), - }, - None => log::error!("Undeclare queryable with unknown scope!"), + } + }; + let mut wtables = zwrite!(tables.tables); + if let Some(mut res) = hat_code.undeclare_queryable(&mut wtables, face, id, res, node_id) { + log::debug!("{} Undeclare queryable {} ({})", face, id, res.expr()); + disable_matches_query_routes(&mut wtables, &mut res); + drop(wtables); + + let rtables = zread!(tables.tables); + let matches_query_routes = compute_matches_query_routes(&rtables, &res); + drop(rtables); + + let wtables = zwrite!(tables.tables); + for (mut res, query_routes) in matches_query_routes { + get_mut_unchecked(&mut res) + .context_mut() + .update_query_routes(query_routes); + } + Resource::clean(&mut res); + drop(wtables); + } else { + log::error!("{} Undeclare unknown queryable {}", face, id); } } @@ -586,7 +616,7 @@ pub fn route_query( ext_tstamp: None, ext_respid: Some(response::ext::ResponderIdType { zid, - eid: 0, // @TODO use proper ResponderId (#703) + eid: 0, // 0 is reserved for routing core }), }, expr.full_expr().to_string(), @@ -701,8 +731,9 @@ pub fn route_query( } None => { log::error!( - "Route query with unknown scope {}! Send final reply.", - expr.scope + "{} Route query with unknown scope {}! Send final reply.", + face, + expr.scope, ); drop(rtables); face.primitives diff --git a/zenoh/src/net/routing/dispatcher/resource.rs b/zenoh/src/net/routing/dispatcher/resource.rs index 813d72a661..9f43841025 100644 --- a/zenoh/src/net/routing/dispatcher/resource.rs +++ b/zenoh/src/net/routing/dispatcher/resource.rs @@ -667,7 +667,11 @@ pub fn register_expr( let mut fullexpr = prefix.expr(); fullexpr.push_str(expr.suffix.as_ref()); if res.expr() != fullexpr { - log::error!("Resource {} remapped. Remapping unsupported!", expr_id); + log::error!( + "{} Resource {} remapped. Remapping unsupported!", + face, + expr_id + ); } } None => { @@ -718,7 +722,11 @@ pub fn register_expr( drop(wtables); } }, - None => log::error!("Declare resource with unknown scope {}!", expr.scope), + None => log::error!( + "{} Declare resource with unknown scope {}!", + face, + expr.scope + ), } } @@ -726,7 +734,7 @@ pub fn unregister_expr(tables: &TablesLock, face: &mut Arc, expr_id: let wtables = zwrite!(tables.tables); match get_mut_unchecked(face).remote_mappings.remove(&expr_id) { Some(mut res) => Resource::clean(&mut res), - None => log::error!("Undeclare unknown resource!"), + None => log::error!("{} Undeclare unknown resource!", face), } drop(wtables); } diff --git a/zenoh/src/net/routing/hat/client/mod.rs b/zenoh/src/net/routing/hat/client/mod.rs index aa83c34f5d..05210bcaee 100644 --- a/zenoh/src/net/routing/hat/client/mod.rs +++ b/zenoh/src/net/routing/hat/client/mod.rs @@ -40,11 +40,11 @@ use super::{ }; use std::{ any::Any, - collections::{HashMap, HashSet}, - sync::Arc, + collections::HashMap, + sync::{atomic::AtomicU32, Arc}, }; use zenoh_config::WhatAmI; -use zenoh_protocol::network::declare::queryable::ext::QueryableInfo; +use zenoh_protocol::network::declare::{queryable::ext::QueryableInfo, QueryableId, SubscriberId}; use zenoh_protocol::network::Oam; use zenoh_result::ZResult; use zenoh_sync::get_mut_unchecked; @@ -131,7 +131,7 @@ impl HatBaseTrait for HatCode { face.local_mappings.clear(); let mut subs_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -159,7 +159,7 @@ impl HatBaseTrait for HatCode { } let mut qabls_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -290,19 +290,21 @@ impl HatContext { } struct HatFace { - local_subs: HashSet>, - remote_subs: HashSet>, - local_qabls: HashMap, QueryableInfo>, - remote_qabls: HashSet>, + next_id: AtomicU32, // @TODO: manage rollover and uniqueness + local_subs: HashMap, SubscriberId>, + remote_subs: HashMap>, + local_qabls: HashMap, (QueryableId, QueryableInfo)>, + remote_qabls: HashMap>, } impl HatFace { fn new() -> Self { Self { - local_subs: HashSet::new(), - remote_subs: HashSet::new(), + next_id: AtomicU32::new(0), + local_subs: HashMap::new(), + remote_subs: HashMap::new(), local_qabls: HashMap::new(), - remote_qabls: HashSet::new(), + remote_qabls: HashMap::new(), } } } diff --git a/zenoh/src/net/routing/hat/client/pubsub.rs b/zenoh/src/net/routing/hat/client/pubsub.rs index 828915018d..f9f827ecc5 100644 --- a/zenoh/src/net/routing/hat/client/pubsub.rs +++ b/zenoh/src/net/routing/hat/client/pubsub.rs @@ -22,8 +22,10 @@ use crate::net::routing::router::RoutesIndexes; use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::SubscriberId; use zenoh_protocol::{ core::{Reliability, WhatAmI}, network::declare::{ @@ -43,10 +45,11 @@ fn propagate_simple_subscription_to( ) { if (src_face.id != dst_face.id || (dst_face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS))) - && !face_hat!(dst_face).local_subs.contains(res) + && !face_hat!(dst_face).local_subs.contains_key(res) && (src_face.whatami == WhatAmI::Client || dst_face.whatami == WhatAmI::Client) { - face_hat_mut!(dst_face).local_subs.insert(res.clone()); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(dst_face).local_subs.insert(res.clone(), id); let key_expr = Resource::decl_key(res, dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -54,7 +57,7 @@ fn propagate_simple_subscription_to( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: *sub_info, }), @@ -83,13 +86,13 @@ fn propagate_simple_subscription( fn register_client_subscription( _tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { // Register subscription { let res = get_mut_unchecked(res); - log::debug!("Register subscription {} for {}", res.expr(), face); match res.session_ctxs.get_mut(&face.id) { Some(ctx) => match &ctx.subs { Some(info) => { @@ -118,16 +121,17 @@ fn register_client_subscription( } } } - face_hat_mut!(face).remote_subs.insert(res.clone()); + face_hat_mut!(face).remote_subs.insert(id, res.clone()); } fn declare_client_subscription( tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { - register_client_subscription(tables, face, res, sub_info); + register_client_subscription(tables, face, id, res, sub_info); let mut propa_sub_info = *sub_info; propa_sub_info.mode = Mode::Push; @@ -144,7 +148,7 @@ fn declare_client_subscription( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id: 0, // @TODO use proper SubscriberId wire_expr: res.expr().into(), ext_info: *sub_info, }), @@ -170,21 +174,19 @@ fn client_subs(res: &Arc) -> Vec> { fn propagate_forget_simple_subscription(tables: &mut Tables, res: &Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_subs.contains(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - face_hat_mut!(face).local_subs.remove(res); } } } @@ -194,45 +196,48 @@ pub(super) fn undeclare_client_subscription( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client subscription {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).subs = None; - } - face_hat_mut!(face).remote_subs.remove(res); - - let mut client_subs = client_subs(res); - if client_subs.is_empty() { - propagate_forget_simple_subscription(tables, res); - } - if client_subs.len() == 1 { - let face = &mut client_subs[0]; - if face_hat!(face).local_subs.contains(res) - && !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) - { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if !face_hat_mut!(face).remote_subs.values().any(|s| *s == *res) { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).subs = None; + } - face_hat_mut!(face).local_subs.remove(res); + let mut client_subs = client_subs(res); + if client_subs.is_empty() { + propagate_forget_simple_subscription(tables, res); + } + if client_subs.len() == 1 { + let face = &mut client_subs[0]; + if !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) { + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } + } } } } + fn forget_client_subscription( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_subscription(tables, face, res); + id: SubscriberId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_subs.remove(&id) { + undeclare_client_subscription(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { @@ -246,7 +251,7 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { .cloned() .collect::>>() { - for sub in &face_hat!(src_face).remote_subs { + for sub in face_hat!(src_face).remote_subs.values() { propagate_simple_subscription_to(tables, face, sub, &sub_info, &mut src_face.clone()); } } @@ -257,27 +262,29 @@ impl HatPubSubTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, _node_id: NodeId, ) { - declare_client_subscription(tables, face, res, sub_info); + declare_client_subscription(tables, face, id, res, sub_info); } fn undeclare_subscription( &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: SubscriberId, + _res: Option>, _node_id: NodeId, - ) { - forget_client_subscription(tables, face, res); + ) -> Option> { + forget_client_subscription(tables, face, id) } fn get_subscriptions(&self, tables: &Tables) -> Vec> { let mut subs = HashSet::new(); for src_face in tables.faces.values() { - for sub in &face_hat!(src_face).remote_subs { + for sub in face_hat!(src_face).remote_subs.values() { subs.insert(sub.clone()); } } diff --git a/zenoh/src/net/routing/hat/client/queries.rs b/zenoh/src/net/routing/hat/client/queries.rs index c6dfc34eac..4964a8880a 100644 --- a/zenoh/src/net/routing/hat/client/queries.rs +++ b/zenoh/src/net/routing/hat/client/queries.rs @@ -23,10 +23,12 @@ use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use ordered_float::OrderedFloat; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_buffers::ZBuf; use zenoh_protocol::core::key_expr::include::{Includer, DEFAULT_INCLUDER}; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::QueryableId; use zenoh_protocol::{ core::{WhatAmI, WireExpr}, network::declare::{ @@ -83,16 +85,19 @@ fn propagate_simple_queryable( let faces = tables.faces.values().cloned(); for mut dst_face in faces { let info = local_qabl_info(tables, res, &dst_face); - let current_info = face_hat!(dst_face).local_qabls.get(res); + let current = face_hat!(dst_face).local_qabls.get(res); if (src_face.is_none() || src_face.as_ref().unwrap().id != dst_face.id) - && (current_info.is_none() || *current_info.unwrap() != info) + && (current.is_none() || current.unwrap().1 != info) && (src_face.is_none() || src_face.as_ref().unwrap().whatami == WhatAmI::Client || dst_face.whatami == WhatAmI::Client) { + let id = current + .map(|c| c.0) + .unwrap_or(face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst)); face_hat_mut!(&mut dst_face) .local_qabls - .insert(res.clone(), info); + .insert(res.clone(), (id, info)); let key_expr = Resource::decl_key(res, &mut dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -100,7 +105,7 @@ fn propagate_simple_queryable( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -114,13 +119,13 @@ fn propagate_simple_queryable( fn register_client_queryable( _tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { // Register queryable { let res = get_mut_unchecked(res); - log::debug!("Register queryable {} (face: {})", res.expr(), face,); get_mut_unchecked(res.session_ctxs.entry(face.id).or_insert_with(|| { Arc::new(SessionContext { face: face.clone(), @@ -135,16 +140,17 @@ fn register_client_queryable( })) .qabl = Some(*qabl_info); } - face_hat_mut!(face).remote_qabls.insert(res.clone()); + face_hat_mut!(face).remote_qabls.insert(id, res.clone()); } fn declare_client_queryable( tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { - register_client_queryable(tables, face, res, qabl_info); + register_client_queryable(tables, face, id, res, qabl_info); propagate_simple_queryable(tables, res, Some(face)); } @@ -164,22 +170,19 @@ fn client_qabls(res: &Arc) -> Vec> { fn propagate_forget_simple_queryable(tables: &mut Tables, res: &mut Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - - face_hat_mut!(face).local_qabls.remove(res); } } } @@ -189,38 +192,37 @@ pub(super) fn undeclare_client_queryable( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client queryable {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).qabl = None; - if ctx.qabl.is_none() { - face_hat_mut!(face).remote_qabls.remove(res); + if !face_hat_mut!(face) + .remote_qabls + .values() + .any(|s| *s == *res) + { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).qabl = None; } - } - let mut client_qabls = client_qabls(res); - if client_qabls.is_empty() { - propagate_forget_simple_queryable(tables, res); - } else { - propagate_simple_queryable(tables, res, None); - } - if client_qabls.len() == 1 { - let face = &mut client_qabls[0]; - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); - - face_hat_mut!(face).local_qabls.remove(res); + let mut client_qabls = client_qabls(res); + if client_qabls.is_empty() { + propagate_forget_simple_queryable(tables, res); + } else { + propagate_simple_queryable(tables, res, None); + } + if client_qabls.len() == 1 { + let face = &mut client_qabls[0]; + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -228,9 +230,14 @@ pub(super) fn undeclare_client_queryable( fn forget_client_queryable( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_queryable(tables, face, res); + id: QueryableId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_qabls.remove(&id) { + undeclare_client_queryable(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn queries_new_face(tables: &mut Tables, _face: &mut Arc) { @@ -240,7 +247,7 @@ pub(super) fn queries_new_face(tables: &mut Tables, _face: &mut Arc) .cloned() .collect::>>() { - for qabl in face_hat!(face).remote_qabls.iter() { + for qabl in face_hat!(face).remote_qabls.values() { propagate_simple_queryable(tables, qabl, Some(&mut face.clone())); } } @@ -255,27 +262,29 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, _node_id: NodeId, ) { - declare_client_queryable(tables, face, res, qabl_info); + declare_client_queryable(tables, face, id, res, qabl_info); } fn undeclare_queryable( &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: QueryableId, + _res: Option>, _node_id: NodeId, - ) { - forget_client_queryable(tables, face, res); + ) -> Option> { + forget_client_queryable(tables, face, id) } fn get_queryables(&self, tables: &Tables) -> Vec> { let mut qabls = HashSet::new(); for src_face in tables.faces.values() { - for qabl in &face_hat!(src_face).remote_qabls { + for qabl in face_hat!(src_face).remote_qabls.values() { qabls.insert(qabl.clone()); } } diff --git a/zenoh/src/net/routing/hat/linkstate_peer/mod.rs b/zenoh/src/net/routing/hat/linkstate_peer/mod.rs index a655d2f0a3..5591ea3b3e 100644 --- a/zenoh/src/net/routing/hat/linkstate_peer/mod.rs +++ b/zenoh/src/net/routing/hat/linkstate_peer/mod.rs @@ -47,12 +47,16 @@ use async_std::task::JoinHandle; use std::{ any::Any, collections::{HashMap, HashSet}, - sync::Arc, + sync::{atomic::AtomicU32, Arc}, }; use zenoh_config::{unwrap_or_default, ModeDependent, WhatAmI, WhatAmIMatcher, ZenohId}; use zenoh_protocol::{ common::ZExtBody, - network::{declare::queryable::ext::QueryableInfo, oam::id::OAM_LINKSTATE, Oam}, + network::{ + declare::{queryable::ext::QueryableInfo, QueryableId, SubscriberId}, + oam::id::OAM_LINKSTATE, + Oam, + }, }; use zenoh_result::ZResult; use zenoh_sync::get_mut_unchecked; @@ -126,7 +130,6 @@ impl HatTables { } fn schedule_compute_trees(&mut self, tables_ref: Arc) { - log::trace!("Schedule computations"); if self.peers_trees_task.is_none() { let task = Some(async_std::task::spawn(async move { async_std::task::sleep(std::time::Duration::from_millis( @@ -142,7 +145,6 @@ impl HatTables { pubsub::pubsub_tree_change(&mut tables, &new_childs); queries::queries_tree_change(&mut tables, &new_childs); - log::trace!("Computations completed"); hat_mut!(tables).peers_trees_task = None; })); self.peers_trees_task = task; @@ -248,7 +250,7 @@ impl HatBaseTrait for HatCode { face.local_mappings.clear(); let mut subs_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -276,7 +278,7 @@ impl HatBaseTrait for HatCode { } let mut qabls_matches = vec![]; - for mut res in face + for (_, mut res) in face .hat .downcast_mut::() .unwrap() @@ -471,20 +473,22 @@ impl HatContext { struct HatFace { link_id: usize, - local_subs: HashSet>, - remote_subs: HashSet>, - local_qabls: HashMap, QueryableInfo>, - remote_qabls: HashSet>, + next_id: AtomicU32, // @TODO: manage rollover and uniqueness + local_subs: HashMap, SubscriberId>, + remote_subs: HashMap>, + local_qabls: HashMap, (QueryableId, QueryableInfo)>, + remote_qabls: HashMap>, } impl HatFace { fn new() -> Self { Self { link_id: 0, - local_subs: HashSet::new(), - remote_subs: HashSet::new(), + next_id: AtomicU32::new(0), + local_subs: HashMap::new(), + remote_subs: HashMap::new(), local_qabls: HashMap::new(), - remote_qabls: HashSet::new(), + remote_qabls: HashMap::new(), } } } diff --git a/zenoh/src/net/routing/hat/linkstate_peer/pubsub.rs b/zenoh/src/net/routing/hat/linkstate_peer/pubsub.rs index c364f7359f..9a41915333 100644 --- a/zenoh/src/net/routing/hat/linkstate_peer/pubsub.rs +++ b/zenoh/src/net/routing/hat/linkstate_peer/pubsub.rs @@ -25,8 +25,10 @@ use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use petgraph::graph::NodeIndex; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::SubscriberId; use zenoh_protocol::{ core::{Reliability, WhatAmI, ZenohId}, network::declare::{ @@ -53,8 +55,6 @@ fn send_sourced_subscription_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let key_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send subscription {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -63,7 +63,7 @@ fn send_sourced_subscription_to_net_childs( node_id: routing_context, }, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // TODO + id: 0, // Sourced subscriptions do not use ids wire_expr: key_expr, ext_info: *sub_info, }), @@ -87,10 +87,11 @@ fn propagate_simple_subscription_to( src_face: &mut Arc, ) { if (src_face.id != dst_face.id || res.expr().starts_with(PREFIX_LIVELINESS)) - && !face_hat!(dst_face).local_subs.contains(res) + && !face_hat!(dst_face).local_subs.contains_key(res) && dst_face.whatami == WhatAmI::Client { - face_hat_mut!(dst_face).local_subs.insert(res.clone()); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(dst_face).local_subs.insert(res.clone(), id); let key_expr = Resource::decl_key(res, dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -98,7 +99,7 @@ fn propagate_simple_subscription_to( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // TODO + id, wire_expr: key_expr, ext_info: *sub_info, }), @@ -171,7 +172,6 @@ fn register_peer_subscription( if !res_hat!(res).peer_subs.contains(&peer) { // Register peer subscription { - log::debug!("Register peer subscription {} (peer: {})", res.expr(), peer); res_hat_mut!(res).peer_subs.insert(peer); hat_mut!(tables).peer_subs.insert(res.clone()); } @@ -199,13 +199,13 @@ fn declare_peer_subscription( fn register_client_subscription( _tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { // Register subscription { let res = get_mut_unchecked(res); - log::debug!("Register subscription {} for {}", res.expr(), face); match res.session_ctxs.get_mut(&face.id) { Some(ctx) => match &ctx.subs { Some(info) => { @@ -234,16 +234,17 @@ fn register_client_subscription( } } } - face_hat_mut!(face).remote_subs.insert(res.clone()); + face_hat_mut!(face).remote_subs.insert(id, res.clone()); } fn declare_client_subscription( tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { - register_client_subscription(tables, face, res, sub_info); + register_client_subscription(tables, face, id, res, sub_info); let mut propa_sub_info = *sub_info; propa_sub_info.mode = Mode::Push; let zid = tables.zid; @@ -289,8 +290,6 @@ fn send_forget_sourced_subscription_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let wire_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send forget subscription {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -299,7 +298,7 @@ fn send_forget_sourced_subscription_to_net_childs( node_id: routing_context.unwrap_or(0), }, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // TODO + id: 0, // Sourced subscriptions do not use ids ext_wire_expr: WireExprType { wire_expr }, }), }, @@ -315,21 +314,19 @@ fn send_forget_sourced_subscription_to_net_childs( fn propagate_forget_simple_subscription(tables: &mut Tables, res: &Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_subs.contains(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // TODO - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - face_hat_mut!(face).local_subs.remove(res); } } } @@ -370,11 +367,6 @@ fn propagate_forget_sourced_subscription( } fn unregister_peer_subscription(tables: &mut Tables, res: &mut Arc, peer: &ZenohId) { - log::debug!( - "Unregister peer subscription {} (peer: {})", - res.expr(), - peer - ); res_hat_mut!(res).peer_subs.retain(|sub| sub != peer); if res_hat!(res).peer_subs.is_empty() { @@ -414,37 +406,34 @@ pub(super) fn undeclare_client_subscription( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client subscription {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).subs = None; - } - face_hat_mut!(face).remote_subs.remove(res); - - let mut client_subs = client_subs(res); - let peer_subs = remote_peer_subs(tables, res); - if client_subs.is_empty() { - undeclare_peer_subscription(tables, None, res, &tables.zid.clone()); - } - if client_subs.len() == 1 && !peer_subs { - let face = &mut client_subs[0]; - if face_hat!(face).local_subs.contains(res) - && !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) - { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // TODO - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if !face_hat_mut!(face).remote_subs.values().any(|s| *s == *res) { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).subs = None; + } - face_hat_mut!(face).local_subs.remove(res); + let mut client_subs = client_subs(res); + let peer_subs = remote_peer_subs(tables, res); + if client_subs.is_empty() { + undeclare_peer_subscription(tables, None, res, &tables.zid.clone()); + } + if client_subs.len() == 1 && !peer_subs { + let face = &mut client_subs[0]; + if !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) { + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } + } } } } @@ -452,20 +441,26 @@ pub(super) fn undeclare_client_subscription( fn forget_client_subscription( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_subscription(tables, face, res); + id: SubscriberId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_subs.remove(&id) { + undeclare_client_subscription(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { let sub_info = SubscriberInfo { - reliability: Reliability::Reliable, // @TODO + reliability: Reliability::Reliable, // @TODO compute proper reliability to propagate from reliability of known subscribers mode: Mode::Push, }; if face.whatami == WhatAmI::Client { for sub in &hat!(tables).peer_subs { - face_hat_mut!(face).local_subs.insert(sub.clone()); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face).local_subs.insert(sub.clone(), id); let key_expr = Resource::decl_key(sub, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -473,7 +468,7 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // TODO + id, wire_expr: key_expr, ext_info: sub_info, }), @@ -515,7 +510,7 @@ pub(super) fn pubsub_tree_change(tables: &mut Tables, new_childs: &[Vec, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, node_id: NodeId, @@ -585,7 +581,7 @@ impl HatPubSubTrait for HatCode { declare_peer_subscription(tables, face, res, sub_info, peer) } } else { - declare_client_subscription(tables, face, res, sub_info) + declare_client_subscription(tables, face, id, res, sub_info) } } @@ -593,15 +589,23 @@ impl HatPubSubTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: SubscriberId, + res: Option>, node_id: NodeId, - ) { + ) -> Option> { if face.whatami != WhatAmI::Client { - if let Some(peer) = get_peer(tables, face, node_id) { - forget_peer_subscription(tables, face, res, &peer); + if let Some(mut res) = res { + if let Some(peer) = get_peer(tables, face, node_id) { + forget_peer_subscription(tables, face, &mut res, &peer); + Some(res) + } else { + None + } + } else { + None } } else { - forget_client_subscription(tables, face, res); + forget_client_subscription(tables, face, id) } } diff --git a/zenoh/src/net/routing/hat/linkstate_peer/queries.rs b/zenoh/src/net/routing/hat/linkstate_peer/queries.rs index 4192f87e55..51aac2175a 100644 --- a/zenoh/src/net/routing/hat/linkstate_peer/queries.rs +++ b/zenoh/src/net/routing/hat/linkstate_peer/queries.rs @@ -26,10 +26,12 @@ use ordered_float::OrderedFloat; use petgraph::graph::NodeIndex; use std::borrow::Cow; use std::collections::HashMap; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_buffers::ZBuf; use zenoh_protocol::core::key_expr::include::{Includer, DEFAULT_INCLUDER}; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::QueryableId; use zenoh_protocol::{ core::{WhatAmI, WireExpr, ZenohId}, network::declare::{ @@ -133,8 +135,6 @@ fn send_sourced_queryable_to_net_childs( if src_face.is_none() || someface.id != src_face.as_ref().unwrap().id { let key_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send queryable {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -143,7 +143,7 @@ fn send_sourced_queryable_to_net_childs( node_id: routing_context, }, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id: 0, // Sourced queryables do not use ids wire_expr: key_expr, ext_info: *qabl_info, }), @@ -166,14 +166,17 @@ fn propagate_simple_queryable( let faces = tables.faces.values().cloned(); for mut dst_face in faces { let info = local_qabl_info(tables, res, &dst_face); - let current_info = face_hat!(dst_face).local_qabls.get(res); + let current = face_hat!(dst_face).local_qabls.get(res); if (src_face.is_none() || src_face.as_ref().unwrap().id != dst_face.id) - && (current_info.is_none() || *current_info.unwrap() != info) + && (current.is_none() || current.unwrap().1 != info) && dst_face.whatami == WhatAmI::Client { + let id = current + .map(|c| c.0) + .unwrap_or(face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst)); face_hat_mut!(&mut dst_face) .local_qabls - .insert(res.clone(), info); + .insert(res.clone(), (id, info)); let key_expr = Resource::decl_key(res, &mut dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -181,7 +184,7 @@ fn propagate_simple_queryable( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -240,7 +243,6 @@ fn register_peer_queryable( if current_info.is_none() || current_info.unwrap() != qabl_info { // Register peer queryable { - log::debug!("Register peer queryable {} (peer: {})", res.expr(), peer,); res_hat_mut!(res).peer_qabls.insert(peer, *qabl_info); hat_mut!(tables).peer_qabls.insert(res.clone()); } @@ -269,13 +271,13 @@ fn declare_peer_queryable( fn register_client_queryable( _tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { // Register queryable { let res = get_mut_unchecked(res); - log::debug!("Register queryable {} (face: {})", res.expr(), face,); get_mut_unchecked(res.session_ctxs.entry(face.id).or_insert_with(|| { Arc::new(SessionContext { face: face.clone(), @@ -290,17 +292,17 @@ fn register_client_queryable( })) .qabl = Some(*qabl_info); } - face_hat_mut!(face).remote_qabls.insert(res.clone()); + face_hat_mut!(face).remote_qabls.insert(id, res.clone()); } fn declare_client_queryable( tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { - register_client_queryable(tables, face, res, qabl_info); - + register_client_queryable(tables, face, id, res, qabl_info); let local_details = local_peer_qabl_info(tables, res); let zid = tables.zid; register_peer_queryable(tables, Some(face), res, &local_details, zid); @@ -345,8 +347,6 @@ fn send_forget_sourced_queryable_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let wire_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send forget queryable {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -355,7 +355,7 @@ fn send_forget_sourced_queryable_to_net_childs( node_id: routing_context, }, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id: 0, ext_wire_expr: WireExprType { wire_expr }, }), }, @@ -371,22 +371,19 @@ fn send_forget_sourced_queryable_to_net_childs( fn propagate_forget_simple_queryable(tables: &mut Tables, res: &mut Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - - face_hat_mut!(face).local_qabls.remove(res); } } } @@ -427,7 +424,6 @@ fn propagate_forget_sourced_queryable( } fn unregister_peer_queryable(tables: &mut Tables, res: &mut Arc, peer: &ZenohId) { - log::debug!("Unregister peer queryable {} (peer: {})", res.expr(), peer,); res_hat_mut!(res).peer_qabls.remove(peer); if res_hat!(res).peer_qabls.is_empty() { @@ -467,42 +463,41 @@ pub(super) fn undeclare_client_queryable( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client queryable {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).qabl = None; - if ctx.qabl.is_none() { - face_hat_mut!(face).remote_qabls.remove(res); + if !face_hat_mut!(face) + .remote_qabls + .values() + .any(|s| *s == *res) + { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).qabl = None; } - } - let mut client_qabls = client_qabls(res); - let peer_qabls = remote_peer_qabls(tables, res); + let mut client_qabls = client_qabls(res); + let peer_qabls = remote_peer_qabls(tables, res); - if client_qabls.is_empty() { - undeclare_peer_queryable(tables, None, res, &tables.zid.clone()); - } else { - let local_info = local_peer_qabl_info(tables, res); - register_peer_queryable(tables, None, res, &local_info, tables.zid); - } - - if client_qabls.len() == 1 && !peer_qabls { - let face = &mut client_qabls[0]; - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if client_qabls.is_empty() { + undeclare_peer_queryable(tables, None, res, &tables.zid.clone()); + } else { + let local_info = local_peer_qabl_info(tables, res); + register_peer_queryable(tables, None, res, &local_info, tables.zid); + } - face_hat_mut!(face).local_qabls.remove(res); + if client_qabls.len() == 1 && !peer_qabls { + let face = &mut client_qabls[0]; + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -510,9 +505,14 @@ pub(super) fn undeclare_client_queryable( fn forget_client_queryable( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_queryable(tables, face, res); + id: QueryableId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_qabls.remove(&id) { + undeclare_client_queryable(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { @@ -520,7 +520,10 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { for qabl in &hat!(tables).peer_qabls { if qabl.context.is_some() { let info = local_qabl_info(tables, qabl, face); - face_hat_mut!(face).local_qabls.insert(qabl.clone(), info); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face) + .local_qabls + .insert(qabl.clone(), (id, info)); let key_expr = Resource::decl_key(qabl, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -528,7 +531,7 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -641,6 +644,7 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, node_id: NodeId, @@ -650,7 +654,7 @@ impl HatQueriesTrait for HatCode { declare_peer_queryable(tables, face, res, qabl_info, peer); } } else { - declare_client_queryable(tables, face, res, qabl_info); + declare_client_queryable(tables, face, id, res, qabl_info); } } @@ -658,15 +662,23 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: QueryableId, + res: Option>, node_id: NodeId, - ) { + ) -> Option> { if face.whatami != WhatAmI::Client { - if let Some(peer) = get_peer(tables, face, node_id) { - forget_peer_queryable(tables, face, res, &peer); + if let Some(mut res) = res { + if let Some(peer) = get_peer(tables, face, node_id) { + forget_peer_queryable(tables, face, &mut res, &peer); + Some(res) + } else { + None + } + } else { + None } } else { - forget_client_queryable(tables, face, res); + forget_client_queryable(tables, face, id) } } diff --git a/zenoh/src/net/routing/hat/mod.rs b/zenoh/src/net/routing/hat/mod.rs index 4fbf9c9e5d..d9feb687f2 100644 --- a/zenoh/src/net/routing/hat/mod.rs +++ b/zenoh/src/net/routing/hat/mod.rs @@ -31,7 +31,10 @@ use zenoh_config::{unwrap_or_default, Config, WhatAmI}; use zenoh_protocol::{ core::WireExpr, network::{ - declare::{queryable::ext::QueryableInfo, subscriber::ext::SubscriberInfo}, + declare::{ + queryable::ext::QueryableInfo, subscriber::ext::SubscriberInfo, QueryableId, + SubscriberId, + }, Oam, }, }; @@ -117,6 +120,7 @@ pub(crate) trait HatPubSubTrait { &self, tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, node_id: NodeId, @@ -125,9 +129,10 @@ pub(crate) trait HatPubSubTrait { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: SubscriberId, + res: Option>, node_id: NodeId, - ); + ) -> Option>; fn get_subscriptions(&self, tables: &Tables) -> Vec>; @@ -147,6 +152,7 @@ pub(crate) trait HatQueriesTrait { &self, tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, node_id: NodeId, @@ -155,9 +161,10 @@ pub(crate) trait HatQueriesTrait { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: QueryableId, + res: Option>, node_id: NodeId, - ); + ) -> Option>; fn get_queryables(&self, tables: &Tables) -> Vec>; diff --git a/zenoh/src/net/routing/hat/p2p_peer/mod.rs b/zenoh/src/net/routing/hat/p2p_peer/mod.rs index 8dc4f15ada..1a6c1ba407 100644 --- a/zenoh/src/net/routing/hat/p2p_peer/mod.rs +++ b/zenoh/src/net/routing/hat/p2p_peer/mod.rs @@ -45,11 +45,14 @@ use super::{ }; use std::{ any::Any, - collections::{HashMap, HashSet}, - sync::Arc, + collections::HashMap, + sync::{atomic::AtomicU32, Arc}, }; use zenoh_config::{unwrap_or_default, ModeDependent, WhatAmI, WhatAmIMatcher}; -use zenoh_protocol::network::Oam; +use zenoh_protocol::network::{ + declare::{QueryableId, SubscriberId}, + Oam, +}; use zenoh_protocol::{ common::ZExtBody, network::{declare::queryable::ext::QueryableInfo, oam::id::OAM_LINKSTATE}, @@ -177,7 +180,7 @@ impl HatBaseTrait for HatCode { face.local_mappings.clear(); let mut subs_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -205,7 +208,7 @@ impl HatBaseTrait for HatCode { } let mut qabls_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -363,19 +366,21 @@ impl HatContext { } struct HatFace { - local_subs: HashSet>, - remote_subs: HashSet>, - local_qabls: HashMap, QueryableInfo>, - remote_qabls: HashSet>, + next_id: AtomicU32, // @TODO: manage rollover and uniqueness + local_subs: HashMap, SubscriberId>, + remote_subs: HashMap>, + local_qabls: HashMap, (QueryableId, QueryableInfo)>, + remote_qabls: HashMap>, } impl HatFace { fn new() -> Self { Self { - local_subs: HashSet::new(), - remote_subs: HashSet::new(), + next_id: AtomicU32::new(0), + local_subs: HashMap::new(), + remote_subs: HashMap::new(), local_qabls: HashMap::new(), - remote_qabls: HashSet::new(), + remote_qabls: HashMap::new(), } } } diff --git a/zenoh/src/net/routing/hat/p2p_peer/pubsub.rs b/zenoh/src/net/routing/hat/p2p_peer/pubsub.rs index a7d58ce1a5..4f6ce5aeca 100644 --- a/zenoh/src/net/routing/hat/p2p_peer/pubsub.rs +++ b/zenoh/src/net/routing/hat/p2p_peer/pubsub.rs @@ -22,8 +22,10 @@ use crate::net::routing::router::RoutesIndexes; use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::SubscriberId; use zenoh_protocol::{ core::{Reliability, WhatAmI}, network::declare::{ @@ -43,10 +45,11 @@ fn propagate_simple_subscription_to( ) { if (src_face.id != dst_face.id || (dst_face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS))) - && !face_hat!(dst_face).local_subs.contains(res) + && !face_hat!(dst_face).local_subs.contains_key(res) && (src_face.whatami == WhatAmI::Client || dst_face.whatami == WhatAmI::Client) { - face_hat_mut!(dst_face).local_subs.insert(res.clone()); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(dst_face).local_subs.insert(res.clone(), id); let key_expr = Resource::decl_key(res, dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -54,7 +57,7 @@ fn propagate_simple_subscription_to( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: *sub_info, }), @@ -83,13 +86,13 @@ fn propagate_simple_subscription( fn register_client_subscription( _tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { // Register subscription { let res = get_mut_unchecked(res); - log::debug!("Register subscription {} for {}", res.expr(), face); match res.session_ctxs.get_mut(&face.id) { Some(ctx) => match &ctx.subs { Some(info) => { @@ -118,16 +121,17 @@ fn register_client_subscription( } } } - face_hat_mut!(face).remote_subs.insert(res.clone()); + face_hat_mut!(face).remote_subs.insert(id, res.clone()); } fn declare_client_subscription( tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { - register_client_subscription(tables, face, res, sub_info); + register_client_subscription(tables, face, id, res, sub_info); let mut propa_sub_info = *sub_info; propa_sub_info.mode = Mode::Push; @@ -144,7 +148,7 @@ fn declare_client_subscription( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id: 0, // @TODO use proper SubscriberId wire_expr: res.expr().into(), ext_info: *sub_info, }), @@ -170,21 +174,19 @@ fn client_subs(res: &Arc) -> Vec> { fn propagate_forget_simple_subscription(tables: &mut Tables, res: &Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_subs.contains(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - face_hat_mut!(face).local_subs.remove(res); } } } @@ -194,36 +196,33 @@ pub(super) fn undeclare_client_subscription( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client subscription {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).subs = None; - } - face_hat_mut!(face).remote_subs.remove(res); - - let mut client_subs = client_subs(res); - if client_subs.is_empty() { - propagate_forget_simple_subscription(tables, res); - } - if client_subs.len() == 1 { - let face = &mut client_subs[0]; - if face_hat!(face).local_subs.contains(res) - && !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) - { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if !face_hat_mut!(face).remote_subs.values().any(|s| *s == *res) { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).subs = None; + } - face_hat_mut!(face).local_subs.remove(res); + let mut client_subs = client_subs(res); + if client_subs.is_empty() { + propagate_forget_simple_subscription(tables, res); + } + if client_subs.len() == 1 { + let face = &mut client_subs[0]; + if !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) { + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } + } } } } @@ -231,9 +230,14 @@ pub(super) fn undeclare_client_subscription( fn forget_client_subscription( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_subscription(tables, face, res); + id: SubscriberId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_subs.remove(&id) { + undeclare_client_subscription(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { @@ -247,7 +251,7 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { .cloned() .collect::>>() { - for sub in &face_hat!(src_face).remote_subs { + for sub in face_hat!(src_face).remote_subs.values() { propagate_simple_subscription_to(tables, face, sub, &sub_info, &mut src_face.clone()); } } @@ -258,27 +262,29 @@ impl HatPubSubTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, _node_id: NodeId, ) { - declare_client_subscription(tables, face, res, sub_info); + declare_client_subscription(tables, face, id, res, sub_info); } fn undeclare_subscription( &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: SubscriberId, + _res: Option>, _node_id: NodeId, - ) { - forget_client_subscription(tables, face, res); + ) -> Option> { + forget_client_subscription(tables, face, id) } fn get_subscriptions(&self, tables: &Tables) -> Vec> { let mut subs = HashSet::new(); for src_face in tables.faces.values() { - for sub in &face_hat!(src_face).remote_subs { + for sub in face_hat!(src_face).remote_subs.values() { subs.insert(sub.clone()); } } diff --git a/zenoh/src/net/routing/hat/p2p_peer/queries.rs b/zenoh/src/net/routing/hat/p2p_peer/queries.rs index 68f2669f6f..04b31b41ef 100644 --- a/zenoh/src/net/routing/hat/p2p_peer/queries.rs +++ b/zenoh/src/net/routing/hat/p2p_peer/queries.rs @@ -23,10 +23,12 @@ use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use ordered_float::OrderedFloat; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_buffers::ZBuf; use zenoh_protocol::core::key_expr::include::{Includer, DEFAULT_INCLUDER}; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::QueryableId; use zenoh_protocol::{ core::{WhatAmI, WireExpr}, network::declare::{ @@ -83,16 +85,19 @@ fn propagate_simple_queryable( let faces = tables.faces.values().cloned(); for mut dst_face in faces { let info = local_qabl_info(tables, res, &dst_face); - let current_info = face_hat!(dst_face).local_qabls.get(res); + let current = face_hat!(dst_face).local_qabls.get(res); if (src_face.is_none() || src_face.as_ref().unwrap().id != dst_face.id) - && (current_info.is_none() || *current_info.unwrap() != info) + && (current.is_none() || current.unwrap().1 != info) && (src_face.is_none() || src_face.as_ref().unwrap().whatami == WhatAmI::Client || dst_face.whatami == WhatAmI::Client) { + let id = current + .map(|c| c.0) + .unwrap_or(face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst)); face_hat_mut!(&mut dst_face) .local_qabls - .insert(res.clone(), info); + .insert(res.clone(), (id, info)); let key_expr = Resource::decl_key(res, &mut dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -100,7 +105,7 @@ fn propagate_simple_queryable( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -114,13 +119,13 @@ fn propagate_simple_queryable( fn register_client_queryable( _tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { // Register queryable { let res = get_mut_unchecked(res); - log::debug!("Register queryable {} (face: {})", res.expr(), face,); get_mut_unchecked(res.session_ctxs.entry(face.id).or_insert_with(|| { Arc::new(SessionContext { face: face.clone(), @@ -135,16 +140,17 @@ fn register_client_queryable( })) .qabl = Some(*qabl_info); } - face_hat_mut!(face).remote_qabls.insert(res.clone()); + face_hat_mut!(face).remote_qabls.insert(id, res.clone()); } fn declare_client_queryable( tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { - register_client_queryable(tables, face, res, qabl_info); + register_client_queryable(tables, face, id, res, qabl_info); propagate_simple_queryable(tables, res, Some(face)); } @@ -164,22 +170,19 @@ fn client_qabls(res: &Arc) -> Vec> { fn propagate_forget_simple_queryable(tables: &mut Tables, res: &mut Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - - face_hat_mut!(face).local_qabls.remove(res); } } } @@ -189,38 +192,37 @@ pub(super) fn undeclare_client_queryable( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client queryable {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).qabl = None; - if ctx.qabl.is_none() { - face_hat_mut!(face).remote_qabls.remove(res); + if !face_hat_mut!(face) + .remote_qabls + .values() + .any(|s| *s == *res) + { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).qabl = None; } - } - let mut client_qabls = client_qabls(res); - if client_qabls.is_empty() { - propagate_forget_simple_queryable(tables, res); - } else { - propagate_simple_queryable(tables, res, None); - } - if client_qabls.len() == 1 { - let face = &mut client_qabls[0]; - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); - - face_hat_mut!(face).local_qabls.remove(res); + let mut client_qabls = client_qabls(res); + if client_qabls.is_empty() { + propagate_forget_simple_queryable(tables, res); + } else { + propagate_simple_queryable(tables, res, None); + } + if client_qabls.len() == 1 { + let face = &mut client_qabls[0]; + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -228,9 +230,14 @@ pub(super) fn undeclare_client_queryable( fn forget_client_queryable( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_queryable(tables, face, res); + id: QueryableId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_qabls.remove(&id) { + undeclare_client_queryable(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn queries_new_face(tables: &mut Tables, _face: &mut Arc) { @@ -240,7 +247,7 @@ pub(super) fn queries_new_face(tables: &mut Tables, _face: &mut Arc) .cloned() .collect::>>() { - for qabl in face_hat!(face).remote_qabls.iter() { + for qabl in face_hat!(face).remote_qabls.values() { propagate_simple_queryable(tables, qabl, Some(&mut face.clone())); } } @@ -255,27 +262,29 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, _node_id: NodeId, ) { - declare_client_queryable(tables, face, res, qabl_info); + declare_client_queryable(tables, face, id, res, qabl_info); } fn undeclare_queryable( &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: QueryableId, + _res: Option>, _node_id: NodeId, - ) { - forget_client_queryable(tables, face, res); + ) -> Option> { + forget_client_queryable(tables, face, id) } fn get_queryables(&self, tables: &Tables) -> Vec> { let mut qabls = HashSet::new(); for src_face in tables.faces.values() { - for qabl in &face_hat!(src_face).remote_qabls { + for qabl in face_hat!(src_face).remote_qabls.values() { qabls.insert(qabl.clone()); } } diff --git a/zenoh/src/net/routing/hat/router/mod.rs b/zenoh/src/net/routing/hat/router/mod.rs index 24c837e8f5..ff576ae271 100644 --- a/zenoh/src/net/routing/hat/router/mod.rs +++ b/zenoh/src/net/routing/hat/router/mod.rs @@ -52,12 +52,16 @@ use std::{ any::Any, collections::{hash_map::DefaultHasher, HashMap, HashSet}, hash::Hasher, - sync::Arc, + sync::{atomic::AtomicU32, Arc}, }; use zenoh_config::{unwrap_or_default, ModeDependent, WhatAmI, WhatAmIMatcher, ZenohId}; use zenoh_protocol::{ common::ZExtBody, - network::{declare::queryable::ext::QueryableInfo, oam::id::OAM_LINKSTATE, Oam}, + network::{ + declare::{queryable::ext::QueryableInfo, QueryableId, SubscriberId}, + oam::id::OAM_LINKSTATE, + Oam, + }, }; use zenoh_result::ZResult; use zenoh_sync::get_mut_unchecked; @@ -232,14 +236,12 @@ impl HatTables { .as_ref() .map(|net| { let links = net.get_links(peer1); - log::debug!("failover_brokering {} {} ({:?})", peer1, peer2, links); HatTables::failover_brokering_to(links, peer2) }) .unwrap_or(false) } fn schedule_compute_trees(&mut self, tables_ref: Arc, net_type: WhatAmI) { - log::trace!("Schedule computations"); if (net_type == WhatAmI::Router && self.routers_trees_task.is_none()) || (net_type == WhatAmI::Peer && self.peers_trees_task.is_none()) { @@ -264,7 +266,6 @@ impl HatTables { pubsub::pubsub_tree_change(&mut tables, &new_childs, net_type); queries::queries_tree_change(&mut tables, &new_childs, net_type); - log::trace!("Computations completed"); match net_type { WhatAmI::Router => hat_mut!(tables).routers_trees_task = None, _ => hat_mut!(tables).peers_trees_task = None, @@ -418,7 +419,7 @@ impl HatBaseTrait for HatCode { face.local_mappings.clear(); let mut subs_matches = vec![]; - for mut res in face + for (_id, mut res) in face .hat .downcast_mut::() .unwrap() @@ -446,7 +447,7 @@ impl HatBaseTrait for HatCode { } let mut qabls_matches = vec![]; - for mut res in face + for (_, mut res) in face .hat .downcast_mut::() .unwrap() @@ -773,20 +774,22 @@ impl HatContext { struct HatFace { link_id: usize, - local_subs: HashSet>, - remote_subs: HashSet>, - local_qabls: HashMap, QueryableInfo>, - remote_qabls: HashSet>, + next_id: AtomicU32, // @TODO: manage rollover and uniqueness + local_subs: HashMap, SubscriberId>, + remote_subs: HashMap>, + local_qabls: HashMap, (QueryableId, QueryableInfo)>, + remote_qabls: HashMap>, } impl HatFace { fn new() -> Self { Self { link_id: 0, - local_subs: HashSet::new(), - remote_subs: HashSet::new(), + next_id: AtomicU32::new(0), + local_subs: HashMap::new(), + remote_subs: HashMap::new(), local_qabls: HashMap::new(), - remote_qabls: HashSet::new(), + remote_qabls: HashMap::new(), } } } diff --git a/zenoh/src/net/routing/hat/router/pubsub.rs b/zenoh/src/net/routing/hat/router/pubsub.rs index 6030269cfa..da1ca66efd 100644 --- a/zenoh/src/net/routing/hat/router/pubsub.rs +++ b/zenoh/src/net/routing/hat/router/pubsub.rs @@ -25,8 +25,10 @@ use crate::net::routing::{RoutingContext, PREFIX_LIVELINESS}; use petgraph::graph::NodeIndex; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::SubscriberId; use zenoh_protocol::{ core::{Reliability, WhatAmI, ZenohId}, network::declare::{ @@ -53,8 +55,6 @@ fn send_sourced_subscription_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let key_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send subscription {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -63,7 +63,7 @@ fn send_sourced_subscription_to_net_childs( node_id: routing_context, }, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id: 0, // Sourced subscriptions do not use ids wire_expr: key_expr, ext_info: *sub_info, }), @@ -89,7 +89,7 @@ fn propagate_simple_subscription_to( ) { if (src_face.id != dst_face.id || (dst_face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS))) - && !face_hat!(dst_face).local_subs.contains(res) + && !face_hat!(dst_face).local_subs.contains_key(res) && if full_peer_net { dst_face.whatami == WhatAmI::Client } else { @@ -99,7 +99,8 @@ fn propagate_simple_subscription_to( || hat!(tables).failover_brokering(src_face.zid, dst_face.zid)) } { - face_hat_mut!(dst_face).local_subs.insert(res.clone()); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(dst_face).local_subs.insert(res.clone(), id); let key_expr = Resource::decl_key(res, dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -107,7 +108,7 @@ fn propagate_simple_subscription_to( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: *sub_info, }), @@ -189,11 +190,6 @@ fn register_router_subscription( if !res_hat!(res).router_subs.contains(&router) { // Register router subscription { - log::debug!( - "Register router subscription {} (router: {})", - res.expr(), - router - ); res_hat_mut!(res).router_subs.insert(router); hat_mut!(tables).router_subs.insert(res.clone()); } @@ -230,7 +226,6 @@ fn register_peer_subscription( if !res_hat!(res).peer_subs.contains(&peer) { // Register peer subscription { - log::debug!("Register peer subscription {} (peer: {})", res.expr(), peer); res_hat_mut!(res).peer_subs.insert(peer); hat_mut!(tables).peer_subs.insert(res.clone()); } @@ -257,13 +252,13 @@ fn declare_peer_subscription( fn register_client_subscription( _tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { // Register subscription { let res = get_mut_unchecked(res); - log::debug!("Register subscription {} for {}", res.expr(), face); match res.session_ctxs.get_mut(&face.id) { Some(ctx) => match &ctx.subs { Some(info) => { @@ -292,16 +287,17 @@ fn register_client_subscription( } } } - face_hat_mut!(face).remote_subs.insert(res.clone()); + face_hat_mut!(face).remote_subs.insert(id, res.clone()); } fn declare_client_subscription( tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, ) { - register_client_subscription(tables, face, res, sub_info); + register_client_subscription(tables, face, id, res, sub_info); let mut propa_sub_info = *sub_info; propa_sub_info.mode = Mode::Push; let zid = tables.zid; @@ -356,8 +352,6 @@ fn send_forget_sourced_subscription_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let wire_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send forget subscription {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -366,7 +360,7 @@ fn send_forget_sourced_subscription_to_net_childs( node_id: routing_context.unwrap_or(0), }, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id: 0, // Sourced subscriptions do not use ids ext_wire_expr: WireExprType { wire_expr }, }), }, @@ -382,21 +376,19 @@ fn send_forget_sourced_subscription_to_net_childs( fn propagate_forget_simple_subscription(tables: &mut Tables, res: &Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_subs.contains(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - face_hat_mut!(face).local_subs.remove(res); } } } @@ -413,7 +405,7 @@ fn propagate_forget_simple_subscription_to_peers(tables: &mut Tables, res: &Arc< .collect::>>() { if face.whatami == WhatAmI::Peer - && face_hat!(face).local_subs.contains(res) + && face_hat!(face).local_subs.contains_key(res) && !res.session_ctxs.values().any(|s| { face.zid != s.face.zid && s.subs.is_some() @@ -422,21 +414,20 @@ fn propagate_forget_simple_subscription_to_peers(tables: &mut Tables, res: &Arc< && hat!(tables).failover_brokering(s.face.zid, face.zid))) }) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); - - face_hat_mut!(&mut face).local_subs.remove(res); + if let Some(id) = face_hat_mut!(&mut face).local_subs.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -479,11 +470,6 @@ fn propagate_forget_sourced_subscription( } fn unregister_router_subscription(tables: &mut Tables, res: &mut Arc, router: &ZenohId) { - log::debug!( - "Unregister router subscription {} (router: {})", - res.expr(), - router - ); res_hat_mut!(res).router_subs.retain(|sub| sub != router); if res_hat!(res).router_subs.is_empty() { @@ -522,11 +508,6 @@ fn forget_router_subscription( } fn unregister_peer_subscription(tables: &mut Tables, res: &mut Arc, peer: &ZenohId) { - log::debug!( - "Unregister peer subscription {} (peer: {})", - res.expr(), - peer - ); res_hat_mut!(res).peer_subs.retain(|sub| sub != peer); if res_hat!(res).peer_subs.is_empty() { @@ -568,40 +549,37 @@ pub(super) fn undeclare_client_subscription( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client subscription {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).subs = None; - } - face_hat_mut!(face).remote_subs.remove(res); - - let mut client_subs = client_subs(res); - let router_subs = remote_router_subs(tables, res); - let peer_subs = remote_peer_subs(tables, res); - if client_subs.is_empty() && !peer_subs { - undeclare_router_subscription(tables, None, res, &tables.zid.clone()); - } else { - propagate_forget_simple_subscription_to_peers(tables, res); - } - if client_subs.len() == 1 && !router_subs && !peer_subs { - let face = &mut client_subs[0]; - if face_hat!(face).local_subs.contains(res) - && !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) - { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if !face_hat_mut!(face).remote_subs.values().any(|s| *s == *res) { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).subs = None; + } - face_hat_mut!(face).local_subs.remove(res); + let mut client_subs = client_subs(res); + let router_subs = remote_router_subs(tables, res); + let peer_subs = remote_peer_subs(tables, res); + if client_subs.is_empty() && !peer_subs { + undeclare_router_subscription(tables, None, res, &tables.zid.clone()); + } else { + propagate_forget_simple_subscription_to_peers(tables, res); + } + if client_subs.len() == 1 && !router_subs && !peer_subs { + let face = &mut client_subs[0]; + if !(face.whatami == WhatAmI::Client && res.expr().starts_with(PREFIX_LIVELINESS)) { + if let Some(id) = face_hat_mut!(face).local_subs.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } + } } } } @@ -609,9 +587,14 @@ pub(super) fn undeclare_client_subscription( fn forget_client_subscription( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_subscription(tables, face, res); + id: SubscriberId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_subs.remove(&id) { + undeclare_client_subscription(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { @@ -622,7 +605,8 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { if face.whatami == WhatAmI::Client { for sub in &hat!(tables).router_subs { - face_hat_mut!(face).local_subs.insert(sub.clone()); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face).local_subs.insert(sub.clone(), id); let key_expr = Resource::decl_key(sub, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -630,7 +614,7 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: sub_info, }), @@ -649,7 +633,8 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { && hat!(tables).failover_brokering(s.face.zid, face.zid))) })) { - face_hat_mut!(face).local_subs.insert(sub.clone()); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face).local_subs.insert(sub.clone(), id); let key_expr = Resource::decl_key(sub, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -657,7 +642,7 @@ pub(super) fn pubsub_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: sub_info, }), @@ -760,7 +745,7 @@ pub(super) fn pubsub_tree_change( pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: &[ZenohId]) { if let Some(src_face) = tables.get_face(zid).cloned() { if hat!(tables).router_peers_failover_brokering && src_face.whatami == WhatAmI::Peer { - for res in &face_hat!(src_face).remote_subs { + for res in face_hat!(src_face).remote_subs.values() { let client_subs = res .session_ctxs .values() @@ -772,7 +757,7 @@ pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: { let dst_face = &mut get_mut_unchecked(ctx).face; if dst_face.whatami == WhatAmI::Peer && src_face.zid != dst_face.zid { - if face_hat!(dst_face).local_subs.contains(res) { + if let Some(id) = face_hat!(dst_face).local_subs.get(res).cloned() { let forget = !HatTables::failover_brokering_to(links, dst_face.zid) && { let ctx_links = hat!(tables) @@ -790,7 +775,6 @@ pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: }) }; if forget { - let wire_expr = Resource::get_best_key(res, "", dst_face.id); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -798,8 +782,8 @@ pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber( UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }, ), }, @@ -810,7 +794,8 @@ pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: } } else if HatTables::failover_brokering_to(links, ctx.face.zid) { let dst_face = &mut get_mut_unchecked(ctx).face; - face_hat_mut!(dst_face).local_subs.insert(res.clone()); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(dst_face).local_subs.insert(res.clone(), id); let key_expr = Resource::decl_key(res, dst_face); let sub_info = SubscriberInfo { reliability: Reliability::Reliable, // @TODO compute proper reliability to propagate from reliability of known subscribers @@ -822,7 +807,7 @@ pub(super) fn pubsub_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id, wire_expr: key_expr, ext_info: sub_info, }), @@ -876,6 +861,7 @@ impl HatPubSubTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: SubscriberId, res: &mut Arc, sub_info: &SubscriberInfo, node_id: NodeId, @@ -892,10 +878,10 @@ impl HatPubSubTrait for HatCode { declare_peer_subscription(tables, face, res, sub_info, peer) } } else { - declare_client_subscription(tables, face, res, sub_info) + declare_client_subscription(tables, face, id, res, sub_info) } } - _ => declare_client_subscription(tables, face, res, sub_info), + _ => declare_client_subscription(tables, face, id, res, sub_info), } } @@ -903,25 +889,40 @@ impl HatPubSubTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: SubscriberId, + res: Option>, node_id: NodeId, - ) { + ) -> Option> { match face.whatami { WhatAmI::Router => { - if let Some(router) = get_router(tables, face, node_id) { - forget_router_subscription(tables, face, res, &router) + if let Some(mut res) = res { + if let Some(router) = get_router(tables, face, node_id) { + forget_router_subscription(tables, face, &mut res, &router); + Some(res) + } else { + None + } + } else { + None } } WhatAmI::Peer => { if hat!(tables).full_net(WhatAmI::Peer) { - if let Some(peer) = get_peer(tables, face, node_id) { - forget_peer_subscription(tables, face, res, &peer) + if let Some(mut res) = res { + if let Some(peer) = get_peer(tables, face, node_id) { + forget_peer_subscription(tables, face, &mut res, &peer); + Some(res) + } else { + None + } + } else { + None } } else { - forget_client_subscription(tables, face, res) + forget_client_subscription(tables, face, id) } } - _ => forget_client_subscription(tables, face, res), + _ => forget_client_subscription(tables, face, id), } } diff --git a/zenoh/src/net/routing/hat/router/queries.rs b/zenoh/src/net/routing/hat/router/queries.rs index 008e71d7af..b76f0adcc6 100644 --- a/zenoh/src/net/routing/hat/router/queries.rs +++ b/zenoh/src/net/routing/hat/router/queries.rs @@ -26,10 +26,12 @@ use ordered_float::OrderedFloat; use petgraph::graph::NodeIndex; use std::borrow::Cow; use std::collections::HashMap; +use std::sync::atomic::Ordering; use std::sync::Arc; use zenoh_buffers::ZBuf; use zenoh_protocol::core::key_expr::include::{Includer, DEFAULT_INCLUDER}; use zenoh_protocol::core::key_expr::OwnedKeyExpr; +use zenoh_protocol::network::declare::QueryableId; use zenoh_protocol::{ core::{WhatAmI, WireExpr, ZenohId}, network::declare::{ @@ -204,8 +206,6 @@ fn send_sourced_queryable_to_net_childs( if src_face.is_none() || someface.id != src_face.as_ref().unwrap().id { let key_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send queryable {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -214,7 +214,7 @@ fn send_sourced_queryable_to_net_childs( node_id: routing_context, }, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id: 0, // Sourced queryables do not use ids wire_expr: key_expr, ext_info: *qabl_info, }), @@ -238,9 +238,9 @@ fn propagate_simple_queryable( let faces = tables.faces.values().cloned(); for mut dst_face in faces { let info = local_qabl_info(tables, res, &dst_face); - let current_info = face_hat!(dst_face).local_qabls.get(res); + let current = face_hat!(dst_face).local_qabls.get(res); if (src_face.is_none() || src_face.as_ref().unwrap().id != dst_face.id) - && (current_info.is_none() || *current_info.unwrap() != info) + && (current.is_none() || current.unwrap().1 != info) && if full_peers_net { dst_face.whatami == WhatAmI::Client } else { @@ -252,9 +252,12 @@ fn propagate_simple_queryable( .failover_brokering(src_face.as_ref().unwrap().zid, dst_face.zid)) } { + let id = current + .map(|c| c.0) + .unwrap_or(face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst)); face_hat_mut!(&mut dst_face) .local_qabls - .insert(res.clone(), info); + .insert(res.clone(), (id, info)); let key_expr = Resource::decl_key(res, &mut dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -262,7 +265,7 @@ fn propagate_simple_queryable( ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -322,11 +325,6 @@ fn register_router_queryable( if current_info.is_none() || current_info.unwrap() != qabl_info { // Register router queryable { - log::debug!( - "Register router queryable {} (router: {})", - res.expr(), - router, - ); res_hat_mut!(res).router_qabls.insert(router, *qabl_info); hat_mut!(tables).router_qabls.insert(res.clone()); } @@ -375,7 +373,6 @@ fn register_peer_queryable( if current_info.is_none() || current_info.unwrap() != qabl_info { // Register peer queryable { - log::debug!("Register peer queryable {} (peer: {})", res.expr(), peer,); res_hat_mut!(res).peer_qabls.insert(peer, *qabl_info); hat_mut!(tables).peer_qabls.insert(res.clone()); } @@ -402,13 +399,13 @@ fn declare_peer_queryable( fn register_client_queryable( _tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { // Register queryable { let res = get_mut_unchecked(res); - log::debug!("Register queryable {} (face: {})", res.expr(), face,); get_mut_unchecked(res.session_ctxs.entry(face.id).or_insert_with(|| { Arc::new(SessionContext { face: face.clone(), @@ -423,16 +420,17 @@ fn register_client_queryable( })) .qabl = Some(*qabl_info); } - face_hat_mut!(face).remote_qabls.insert(res.clone()); + face_hat_mut!(face).remote_qabls.insert(id, res.clone()); } fn declare_client_queryable( tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, ) { - register_client_queryable(tables, face, res, qabl_info); + register_client_queryable(tables, face, id, res, qabl_info); let local_details = local_router_qabl_info(tables, res); let zid = tables.zid; register_router_queryable(tables, Some(face), res, &local_details, zid); @@ -486,8 +484,6 @@ fn send_forget_sourced_queryable_to_net_childs( if src_face.is_none() || someface.id != src_face.unwrap().id { let wire_expr = Resource::decl_key(res, &mut someface); - log::debug!("Send forget queryable {} on {}", res.expr(), someface); - someface.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -496,7 +492,7 @@ fn send_forget_sourced_queryable_to_net_childs( node_id: routing_context, }, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id: 0, // Sourced queryables do not use ids ext_wire_expr: WireExprType { wire_expr }, }), }, @@ -512,22 +508,19 @@ fn send_forget_sourced_queryable_to_net_childs( fn propagate_forget_simple_queryable(tables: &mut Tables, res: &mut Arc) { for face in tables.faces.values_mut() { - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }), }, res.expr(), )); - - face_hat_mut!(face).local_qabls.remove(res); } } } @@ -553,21 +546,20 @@ fn propagate_forget_simple_queryable_to_peers(tables: &mut Tables, res: &mut Arc && hat!(tables).failover_brokering(s.face.zid, face.zid))) }) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); - - face_hat_mut!(&mut face).local_qabls.remove(res); + if let Some((id, _)) = face_hat_mut!(&mut face).local_qabls.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -610,11 +602,6 @@ fn propagate_forget_sourced_queryable( } fn unregister_router_queryable(tables: &mut Tables, res: &mut Arc, router: &ZenohId) { - log::debug!( - "Unregister router queryable {} (router: {})", - res.expr(), - router, - ); res_hat_mut!(res).router_qabls.remove(router); if res_hat!(res).router_qabls.is_empty() { @@ -653,7 +640,6 @@ fn forget_router_queryable( } fn unregister_peer_queryable(tables: &mut Tables, res: &mut Arc, peer: &ZenohId) { - log::debug!("Unregister peer queryable {} (peer: {})", res.expr(), peer,); res_hat_mut!(res).peer_qabls.remove(peer); if res_hat!(res).peer_qabls.is_empty() { @@ -699,44 +685,43 @@ pub(super) fn undeclare_client_queryable( face: &mut Arc, res: &mut Arc, ) { - log::debug!("Unregister client queryable {} for {}", res.expr(), face); - if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { - get_mut_unchecked(ctx).qabl = None; - if ctx.qabl.is_none() { - face_hat_mut!(face).remote_qabls.remove(res); + if !face_hat_mut!(face) + .remote_qabls + .values() + .any(|s| *s == *res) + { + if let Some(ctx) = get_mut_unchecked(res).session_ctxs.get_mut(&face.id) { + get_mut_unchecked(ctx).qabl = None; } - } - let mut client_qabls = client_qabls(res); - let router_qabls = remote_router_qabls(tables, res); - let peer_qabls = remote_peer_qabls(tables, res); + let mut client_qabls = client_qabls(res); + let router_qabls = remote_router_qabls(tables, res); + let peer_qabls = remote_peer_qabls(tables, res); - if client_qabls.is_empty() && !peer_qabls { - undeclare_router_queryable(tables, None, res, &tables.zid.clone()); - } else { - let local_info = local_router_qabl_info(tables, res); - register_router_queryable(tables, None, res, &local_info, tables.zid); - propagate_forget_simple_queryable_to_peers(tables, res); - } - - if client_qabls.len() == 1 && !router_qabls && !peer_qabls { - let face = &mut client_qabls[0]; - if face_hat!(face).local_qabls.contains_key(res) { - let wire_expr = Resource::get_best_key(res, "", face.id); - face.primitives.send_declare(RoutingContext::with_expr( - Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }, - res.expr(), - )); + if client_qabls.is_empty() && !peer_qabls { + undeclare_router_queryable(tables, None, res, &tables.zid.clone()); + } else { + let local_info = local_router_qabl_info(tables, res); + register_router_queryable(tables, None, res, &local_info, tables.zid); + propagate_forget_simple_queryable_to_peers(tables, res); + } - face_hat_mut!(face).local_qabls.remove(res); + if client_qabls.len() == 1 && !router_qabls && !peer_qabls { + let face = &mut client_qabls[0]; + if let Some((id, _)) = face_hat_mut!(face).local_qabls.remove(res) { + face.primitives.send_declare(RoutingContext::with_expr( + Declare { + ext_qos: ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id, + ext_wire_expr: WireExprType::null(), + }), + }, + res.expr(), + )); + } } } } @@ -744,9 +729,14 @@ pub(super) fn undeclare_client_queryable( fn forget_client_queryable( tables: &mut Tables, face: &mut Arc, - res: &mut Arc, -) { - undeclare_client_queryable(tables, face, res); + id: QueryableId, +) -> Option> { + if let Some(mut res) = face_hat_mut!(face).remote_qabls.remove(&id) { + undeclare_client_queryable(tables, face, &mut res); + Some(res) + } else { + None + } } pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { @@ -754,7 +744,10 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { for qabl in hat!(tables).router_qabls.iter() { if qabl.context.is_some() { let info = local_qabl_info(tables, qabl, face); - face_hat_mut!(face).local_qabls.insert(qabl.clone(), info); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face) + .local_qabls + .insert(qabl.clone(), (id, info)); let key_expr = Resource::decl_key(qabl, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -762,7 +755,7 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -783,7 +776,10 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { })) { let info = local_qabl_info(tables, qabl, face); - face_hat_mut!(face).local_qabls.insert(qabl.clone(), info); + let id = face_hat!(face).next_id.fetch_add(1, Ordering::SeqCst); + face_hat_mut!(face) + .local_qabls + .insert(qabl.clone(), (id, info)); let key_expr = Resource::decl_key(qabl, face); face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -791,7 +787,7 @@ pub(super) fn queries_new_face(tables: &mut Tables, face: &mut Arc) { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -853,7 +849,7 @@ pub(super) fn queries_remove_node(tables: &mut Tables, node: &ZenohId, net_type: pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links: &[ZenohId]) { if let Some(src_face) = tables.get_face(zid) { if hat!(tables).router_peers_failover_brokering && src_face.whatami == WhatAmI::Peer { - for res in &face_hat!(src_face).remote_qabls { + for res in face_hat!(src_face).remote_qabls.values() { let client_qabls = res .session_ctxs .values() @@ -865,7 +861,7 @@ pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links { let dst_face = &mut get_mut_unchecked(ctx).face; if dst_face.whatami == WhatAmI::Peer && src_face.zid != dst_face.zid { - if face_hat!(dst_face).local_qabls.contains_key(res) { + if let Some(id) = face_hat!(dst_face).local_subs.get(res).cloned() { let forget = !HatTables::failover_brokering_to(links, dst_face.zid) && { let ctx_links = hat!(tables) @@ -883,7 +879,6 @@ pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links }) }; if forget { - let wire_expr = Resource::get_best_key(res, "", dst_face.id); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { ext_qos: ext::QoSType::DECLARE, @@ -891,8 +886,8 @@ pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareQueryable( UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { wire_expr }, + id, + ext_wire_expr: WireExprType::null(), }, ), }, @@ -904,9 +899,10 @@ pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links } else if HatTables::failover_brokering_to(links, ctx.face.zid) { let dst_face = &mut get_mut_unchecked(ctx).face; let info = local_qabl_info(tables, res, dst_face); + let id = face_hat!(dst_face).next_id.fetch_add(1, Ordering::SeqCst); face_hat_mut!(dst_face) .local_qabls - .insert(res.clone(), info); + .insert(res.clone(), (id, info)); let key_expr = Resource::decl_key(res, dst_face); dst_face.primitives.send_declare(RoutingContext::with_expr( Declare { @@ -914,7 +910,7 @@ pub(super) fn queries_linkstate_change(tables: &mut Tables, zid: &ZenohId, links ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id, wire_expr: key_expr, ext_info: info, }), @@ -1024,6 +1020,7 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, + id: QueryableId, res: &mut Arc, qabl_info: &QueryableInfo, node_id: NodeId, @@ -1040,10 +1037,10 @@ impl HatQueriesTrait for HatCode { declare_peer_queryable(tables, face, res, qabl_info, peer) } } else { - declare_client_queryable(tables, face, res, qabl_info) + declare_client_queryable(tables, face, id, res, qabl_info) } } - _ => declare_client_queryable(tables, face, res, qabl_info), + _ => declare_client_queryable(tables, face, id, res, qabl_info), } } @@ -1051,25 +1048,40 @@ impl HatQueriesTrait for HatCode { &self, tables: &mut Tables, face: &mut Arc, - res: &mut Arc, + id: QueryableId, + res: Option>, node_id: NodeId, - ) { + ) -> Option> { match face.whatami { WhatAmI::Router => { - if let Some(router) = get_router(tables, face, node_id) { - forget_router_queryable(tables, face, res, &router) + if let Some(mut res) = res { + if let Some(router) = get_router(tables, face, node_id) { + forget_router_queryable(tables, face, &mut res, &router); + Some(res) + } else { + None + } + } else { + None } } WhatAmI::Peer => { if hat!(tables).full_net(WhatAmI::Peer) { - if let Some(peer) = get_peer(tables, face, node_id) { - forget_peer_queryable(tables, face, res, &peer) + if let Some(mut res) = res { + if let Some(peer) = get_peer(tables, face, node_id) { + forget_peer_queryable(tables, face, &mut res, &peer); + Some(res) + } else { + None + } + } else { + None } } else { - forget_client_queryable(tables, face, res) + forget_client_queryable(tables, face, id) } } - _ => forget_client_queryable(tables, face, res), + _ => forget_client_queryable(tables, face, id), } } diff --git a/zenoh/src/net/runtime/adminspace.rs b/zenoh/src/net/runtime/adminspace.rs index 03b447aae0..e76475f447 100644 --- a/zenoh/src/net/runtime/adminspace.rs +++ b/zenoh/src/net/runtime/adminspace.rs @@ -32,6 +32,7 @@ use std::sync::Mutex; use zenoh_buffers::buffer::SplitBuffer; use zenoh_config::{ConfigValidator, ValidatedMap, WhatAmI}; use zenoh_plugin_trait::{PluginControl, PluginStatus}; +use zenoh_protocol::network::declare::QueryableId; use zenoh_protocol::{ core::{ key_expr::{keyexpr, OwnedKeyExpr}, @@ -59,6 +60,7 @@ type Handler = Arc; pub struct AdminSpace { zid: ZenohId, + queryable_id: QueryableId, primitives: Mutex>>, mappings: Mutex>, handlers: HashMap, @@ -189,6 +191,7 @@ impl AdminSpace { }); let admin = Arc::new(AdminSpace { zid: runtime.zid(), + queryable_id: runtime.next_id(), primitives: Mutex::new(None), mappings: Mutex::new(HashMap::new()), handlers, @@ -278,7 +281,7 @@ impl AdminSpace { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) + id: runtime.next_id(), wire_expr: [&root_key, "/**"].concat().into(), ext_info: QueryableInfo { complete: 0, @@ -292,7 +295,7 @@ impl AdminSpace { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) + id: runtime.next_id(), wire_expr: [&root_key, "/config/**"].concat().into(), ext_info: SubscriberInfo::DEFAULT, }), @@ -431,6 +434,7 @@ impl Primitives for AdminSpace { #[cfg(feature = "unstable")] attachment: query.ext_attachment.map(Into::into), }), + eid: self.queryable_id, }; for (key, handler) in &self.handlers { diff --git a/zenoh/src/net/runtime/mod.rs b/zenoh/src/net/runtime/mod.rs index 7061b38622..8b116b1080 100644 --- a/zenoh/src/net/runtime/mod.rs +++ b/zenoh/src/net/runtime/mod.rs @@ -30,6 +30,7 @@ use async_std::task::JoinHandle; use futures::stream::StreamExt; use futures::Future; use std::any::Any; +use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; use stop_token::future::FutureExt; use stop_token::{StopSource, TimedOutError}; @@ -48,6 +49,7 @@ use zenoh_transport::{ struct RuntimeState { zid: ZenohId, whatami: WhatAmI, + next_id: AtomicU32, metadata: serde_json::Value, router: Arc, config: Notifier, @@ -114,6 +116,7 @@ impl Runtime { state: Arc::new(RuntimeState { zid, whatami, + next_id: AtomicU32::new(1), // 0 is reserved for routing core metadata, router, config: config.clone(), @@ -154,6 +157,11 @@ impl Runtime { zwrite!(self.state.transport_handlers).push(handler); } + #[inline] + pub fn next_id(&self) -> u32 { + self.state.next_id.fetch_add(1, Ordering::SeqCst) + } + pub async fn close(&self) -> ZResult<()> { log::trace!("Runtime::close())"); drop(self.state.stop_source.write().unwrap().take()); diff --git a/zenoh/src/net/tests/tables.rs b/zenoh/src/net/tests/tables.rs index 80a9dd458a..4560eefaae 100644 --- a/zenoh/src/net/tests/tables.rs +++ b/zenoh/src/net/tests/tables.rs @@ -66,6 +66,7 @@ fn base_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face.upgrade().unwrap(), + 0, &WireExpr::from(1).with_suffix("four/five"), &sub_info, NodeId::default(), @@ -166,6 +167,76 @@ fn match_test() { } } +#[test] +fn multisub_test() { + let config = Config::default(); + let router = Router::new( + ZenohId::try_from([1]).unwrap(), + WhatAmI::Client, + Some(Arc::new(HLC::default())), + &config, + ) + .unwrap(); + let tables = router.tables.clone(); + + let primitives = Arc::new(DummyPrimitives {}); + let face0 = Arc::downgrade(&router.new_primitives(primitives).state); + assert!(face0.upgrade().is_some()); + + // -------------- + let sub_info = SubscriberInfo { + reliability: Reliability::Reliable, + mode: Mode::Push, + }; + declare_subscription( + zlock!(tables.ctrl_lock).as_ref(), + &tables, + &mut face0.upgrade().unwrap(), + 0, + &"sub".into(), + &sub_info, + NodeId::default(), + ); + let optres = Resource::get_resource(zread!(tables.tables)._get_root(), "sub") + .map(|res| Arc::downgrade(&res)); + assert!(optres.is_some()); + let res = optres.unwrap(); + assert!(res.upgrade().is_some()); + + declare_subscription( + zlock!(tables.ctrl_lock).as_ref(), + &tables, + &mut face0.upgrade().unwrap(), + 1, + &"sub".into(), + &sub_info, + NodeId::default(), + ); + assert!(res.upgrade().is_some()); + + undeclare_subscription( + zlock!(tables.ctrl_lock).as_ref(), + &tables, + &mut face0.upgrade().unwrap(), + 0, + &WireExpr::empty(), + NodeId::default(), + ); + assert!(res.upgrade().is_some()); + + undeclare_subscription( + zlock!(tables.ctrl_lock).as_ref(), + &tables, + &mut face0.upgrade().unwrap(), + 1, + &WireExpr::empty(), + NodeId::default(), + ); + assert!(res.upgrade().is_none()); + + tables::close_face(&tables, &face0); +} + #[test] fn clean_test() { let config = Config::default(); @@ -241,6 +312,7 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 0, &"todrop1/todrop11".into(), &sub_info, NodeId::default(), @@ -255,6 +327,7 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 1, &WireExpr::from(1).with_suffix("/todrop12"), &sub_info, NodeId::default(), @@ -270,7 +343,8 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), - &WireExpr::from(1).with_suffix("/todrop12"), + 1, + &WireExpr::empty(), NodeId::default(), ); @@ -284,7 +358,8 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), - &"todrop1/todrop11".into(), + 0, + &WireExpr::empty(), NodeId::default(), ); assert!(res1.upgrade().is_some()); @@ -302,6 +377,7 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 2, &"todrop3".into(), &sub_info, NodeId::default(), @@ -316,7 +392,8 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), - &"todrop3".into(), + 2, + &WireExpr::empty(), NodeId::default(), ); assert!(res1.upgrade().is_some()); @@ -331,6 +408,7 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 3, &"todrop5".into(), &sub_info, NodeId::default(), @@ -339,6 +417,7 @@ fn clean_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 4, &"todrop6".into(), &sub_info, NodeId::default(), @@ -518,6 +597,7 @@ fn client_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face0.upgrade().unwrap(), + 0, &WireExpr::from(11).with_suffix("/**"), &sub_info, NodeId::default(), @@ -565,6 +645,7 @@ fn client_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face1.upgrade().unwrap(), + 0, &WireExpr::from(21).with_suffix("/**"), &sub_info, NodeId::default(), @@ -612,6 +693,7 @@ fn client_test() { zlock!(tables.ctrl_lock).as_ref(), &tables, &mut face2.upgrade().unwrap(), + 0, &WireExpr::from(31).with_suffix("/**"), &sub_info, NodeId::default(), diff --git a/zenoh/src/prelude.rs b/zenoh/src/prelude.rs index 59a4bbd96e..177906e9b1 100644 --- a/zenoh/src/prelude.rs +++ b/zenoh/src/prelude.rs @@ -31,7 +31,10 @@ pub(crate) mod common { writer::HasWriter, }; pub use zenoh_core::Resolve; + pub use zenoh_protocol::core::{EndPoint, Locator, ZenohId}; + #[zenoh_macros::unstable] + pub use zenoh_protocol::core::{EntityGlobalId, EntityId}; pub use crate::config::{self, Config, ValidatedMap}; pub use crate::handlers::IntoCallbackReceiverPair; @@ -49,6 +52,8 @@ pub(crate) mod common { pub use crate::sample::Locality; #[cfg(not(feature = "unstable"))] pub(crate) use crate::sample::Locality; + #[zenoh_macros::unstable] + pub use crate::sample::SourceInfo; pub use crate::sample::{Sample, SampleKind}; pub use crate::publication::Priority; diff --git a/zenoh/src/publication.rs b/zenoh/src/publication.rs index 9fb4bdf6c3..2a1a58ebd9 100644 --- a/zenoh/src/publication.rs +++ b/zenoh/src/publication.rs @@ -13,14 +13,11 @@ // //! Publishing primitives. -use crate::encoding::Encoding; -use crate::key_expr::KeyExpr; use crate::net::primitives::Primitives; -use crate::payload::Payload; +use crate::prelude::*; #[zenoh_macros::unstable] use crate::sample::Attachment; use crate::sample::{DataInfo, QoS, Sample, SampleKind}; -use crate::Locality; use crate::SessionRef; use crate::Undeclarable; #[cfg(feature = "unstable")] @@ -30,10 +27,11 @@ use crate::{ }; use std::future::Ready; use zenoh_core::{zread, AsyncResolve, Resolvable, Resolve, SyncResolve}; -use zenoh_keyexpr::keyexpr; use zenoh_protocol::network::push::ext; use zenoh_protocol::network::Mapping; use zenoh_protocol::network::Push; +#[zenoh_macros::unstable] +use zenoh_protocol::zenoh::ext::SourceInfoType; use zenoh_protocol::zenoh::Del; use zenoh_protocol::zenoh::PushBody; use zenoh_protocol::zenoh::Put; @@ -148,6 +146,8 @@ impl SyncResolve for PutBuilder<'_, '_> { let publisher = Publisher { session, + #[cfg(feature = "unstable")] + eid: 0, // This is a one shot Publisher key_expr: key_expr?, congestion_control, priority, @@ -160,6 +160,8 @@ impl SyncResolve for PutBuilder<'_, '_> { self.kind, self.encoding, #[cfg(feature = "unstable")] + None, + #[cfg(feature = "unstable")] self.attachment, ) } @@ -241,6 +243,8 @@ impl std::fmt::Debug for PublisherRef<'_> { #[derive(Debug, Clone)] pub struct Publisher<'a> { pub(crate) session: SessionRef<'a>, + #[cfg(feature = "unstable")] + pub(crate) eid: EntityId, pub(crate) key_expr: KeyExpr<'a>, pub(crate) congestion_control: CongestionControl, pub(crate) priority: Priority, @@ -248,6 +252,29 @@ pub struct Publisher<'a> { } impl<'a> Publisher<'a> { + /// Returns the [`EntityGlobalId`] of this Publisher. + /// + /// # Examples + /// ``` + /// # async_std::task::block_on(async { + /// use zenoh::prelude::r#async::*; + /// + /// let session = zenoh::open(config::peer()).res().await.unwrap(); + /// let publisher = session.declare_publisher("key/expression") + /// .res() + /// .await + /// .unwrap(); + /// let publisher_id = publisher.id(); + /// # }) + /// ``` + #[zenoh_macros::unstable] + pub fn id(&self) -> EntityGlobalId { + EntityGlobalId { + zid: self.session.zid(), + eid: self.eid, + } + } + pub fn key_expr(&self) -> &KeyExpr<'a> { &self.key_expr } @@ -317,6 +344,8 @@ impl<'a> Publisher<'a> { kind, encoding: Encoding::ZENOH_BYTES, #[cfg(feature = "unstable")] + source_info: None, + #[cfg(feature = "unstable")] attachment: None, } } @@ -604,6 +633,8 @@ pub struct Publication<'a> { kind: SampleKind, encoding: Encoding, #[cfg(feature = "unstable")] + pub(crate) source_info: Option, + #[cfg(feature = "unstable")] pub(crate) attachment: Option, } @@ -618,6 +649,27 @@ impl<'a> Publication<'a> { self.attachment = Some(attachment); self } + + /// Send data with the given [`SourceInfo`]. + /// + /// # Examples + /// ``` + /// # async_std::task::block_on(async { + /// use zenoh::prelude::r#async::*; + /// + /// let session = zenoh::open(config::peer()).res().await.unwrap(); + /// let publisher = session.declare_publisher("key/expression").res().await.unwrap(); + /// publisher.put("Value").with_source_info(SourceInfo { + /// source_id: Some(publisher.id()), + /// source_sn: Some(0), + /// }).res().await.unwrap(); + /// # }) + /// ``` + #[zenoh_macros::unstable] + pub fn with_source_info(mut self, source_info: SourceInfo) -> Self { + self.source_info = Some(source_info); + self + } } impl Resolvable for Publication<'_> { @@ -632,6 +684,8 @@ impl SyncResolve for Publication<'_> { self.kind, self.encoding, #[cfg(feature = "unstable")] + self.source_info, + #[cfg(feature = "unstable")] self.attachment, ) } @@ -661,6 +715,8 @@ impl<'a> Sink for Publisher<'a> { kind: item.kind, encoding: item.encoding, #[cfg(feature = "unstable")] + source_info: None, + #[cfg(feature = "unstable")] attachment: item.attachment, } .res_sync() @@ -784,8 +840,12 @@ impl<'a, 'b> SyncResolve for PublisherBuilder<'a, 'b> { self.session .declare_publication_intent(key_expr.clone()) .res_sync()?; + #[cfg(feature = "unstable")] + let eid = self.session.runtime.next_id(); let publisher = Publisher { session: self.session, + #[cfg(feature = "unstable")] + eid, key_expr, congestion_control: self.congestion_control, priority: self.priority, @@ -809,6 +869,7 @@ fn resolve_put( payload: Payload, kind: SampleKind, encoding: Encoding, + #[cfg(feature = "unstable")] source_info: Option, #[cfg(feature = "unstable")] attachment: Option, ) -> ZResult<()> { log::trace!("write({:?}, [...])", &publisher.key_expr); @@ -842,6 +903,12 @@ fn resolve_put( PushBody::Put(Put { timestamp, encoding: encoding.clone().into(), + #[cfg(feature = "unstable")] + ext_sinfo: source_info.map(|s| SourceInfoType { + id: s.source_id.unwrap_or_default(), + sn: s.source_sn.unwrap_or_default() as u32, + }), + #[cfg(not(feature = "unstable"))] ext_sinfo: None, #[cfg(feature = "shared-memory")] ext_shm: None, @@ -861,6 +928,12 @@ fn resolve_put( } PushBody::Del(Del { timestamp, + #[cfg(feature = "unstable")] + ext_sinfo: source_info.map(|s| SourceInfoType { + id: s.source_id.unwrap_or_default(), + sn: s.source_sn.unwrap_or_default() as u32, + }), + #[cfg(not(feature = "unstable"))] ext_sinfo: None, ext_attachment, ext_unknown: vec![], diff --git a/zenoh/src/queryable.rs b/zenoh/src/queryable.rs index 6bd78d4fc7..bd5ec81101 100644 --- a/zenoh/src/queryable.rs +++ b/zenoh/src/queryable.rs @@ -17,7 +17,6 @@ use crate::handlers::{locked, DefaultHandler}; use crate::net::primitives::Primitives; use crate::prelude::*; -use crate::sample::DataInfo; use crate::Id; use crate::SessionRef; use crate::Undeclarable; @@ -28,11 +27,9 @@ use std::future::Ready; use std::ops::Deref; use std::sync::Arc; use zenoh_core::{AsyncResolve, Resolvable, SyncResolve}; -use zenoh_protocol::{ - core::WireExpr, - network::{response, Mapping, RequestId, Response, ResponseFinal}, - zenoh::{self, ext::ValueType, reply::ReplyBody, Del, Put, ResponseBody}, -}; +use zenoh_protocol::core::{EntityId, WireExpr}; +use zenoh_protocol::network::{response, Mapping, RequestId, Response, ResponseFinal}; +use zenoh_protocol::zenoh::{self, ext::ValueType, reply::ReplyBody, Del, Put, ResponseBody}; use zenoh_result::ZResult; pub(crate) struct QueryInner { @@ -64,6 +61,7 @@ impl Drop for QueryInner { #[derive(Clone)] pub struct Query { pub(crate) inner: Arc, + pub(crate) eid: EntityId, } impl Query { @@ -192,22 +190,12 @@ impl SyncResolve for ReplyBuilder<'_> { kind, encoding, timestamp, - qos, #[cfg(feature = "unstable")] source_info, #[cfg(feature = "unstable")] attachment, + .. } = sample; - #[allow(unused_mut)] - let mut data_info = DataInfo { - kind, - encoding: Some(encoding), - timestamp, - qos, - source_id: None, - source_sn: None, - }; - // Use a macro for inferring the proper const extension ID between Put and Del cases macro_rules! ext_attachment { () => {{ @@ -222,21 +210,17 @@ impl SyncResolve for ReplyBuilder<'_> { ext_attachment }}; } - + #[allow(unused_mut)] + let mut ext_sinfo = None; #[cfg(feature = "unstable")] { - data_info.source_id = source_info.source_id; - data_info.source_sn = source_info.source_sn; + if source_info.source_id.is_some() || source_info.source_sn.is_some() { + ext_sinfo = Some(zenoh::put::ext::SourceInfoType { + id: source_info.source_id.unwrap_or_default(), + sn: source_info.source_sn.unwrap_or_default() as u32, + }) + } } - let ext_sinfo = if data_info.source_id.is_some() || data_info.source_sn.is_some() { - Some(zenoh::put::ext::SourceInfoType { - zid: data_info.source_id.unwrap_or_default(), - eid: 0, // @TODO use proper EntityId (#703) - sn: data_info.source_sn.unwrap_or_default() as u32, - }) - } else { - None - }; self.query.inner.primitives.send_response(Response { rid: self.query.inner.qid, wire_expr: WireExpr { @@ -249,8 +233,8 @@ impl SyncResolve for ReplyBuilder<'_> { ext_unknown: vec![], payload: match kind { SampleKind::Put => ReplyBody::Put(Put { - timestamp: data_info.timestamp, - encoding: data_info.encoding.unwrap_or_default().into(), + timestamp, + encoding: encoding.into(), ext_sinfo, #[cfg(feature = "shared-memory")] ext_shm: None, @@ -270,7 +254,7 @@ impl SyncResolve for ReplyBuilder<'_> { ext_tstamp: None, ext_respid: Some(response::ext::ResponderIdType { zid: self.query.inner.zid, - eid: 0, // @TODO use proper EntityId (#703) + eid: self.query.eid, }), }); Ok(()) @@ -300,7 +284,7 @@ impl SyncResolve for ReplyBuilder<'_> { ext_tstamp: None, ext_respid: Some(response::ext::ResponderIdType { zid: self.query.inner.zid, - eid: 0, // @TODO use proper EntityId (#703) + eid: self.query.eid, }), }); Ok(()) @@ -607,6 +591,29 @@ pub struct Queryable<'a, Receiver> { } impl<'a, Receiver> Queryable<'a, Receiver> { + /// Returns the [`EntityGlobalId`] of this Queryable. + /// + /// # Examples + /// ``` + /// # async_std::task::block_on(async { + /// use zenoh::prelude::r#async::*; + /// + /// let session = zenoh::open(config::peer()).res().await.unwrap(); + /// let queryable = session.declare_queryable("key/expression") + /// .res() + /// .await + /// .unwrap(); + /// let queryable_id = queryable.id(); + /// # }) + /// ``` + #[zenoh_macros::unstable] + pub fn id(&self) -> EntityGlobalId { + EntityGlobalId { + zid: self.queryable.session.zid(), + eid: self.queryable.state.id, + } + } + #[inline] pub fn undeclare(self) -> impl Resolve> + 'a { Undeclarable::undeclare_inner(self, ()) diff --git a/zenoh/src/sample.rs b/zenoh/src/sample.rs index 543dd62e84..af4a58956d 100644 --- a/zenoh/src/sample.rs +++ b/zenoh/src/sample.rs @@ -15,16 +15,16 @@ //! Sample primitives use crate::encoding::Encoding; use crate::payload::Payload; -use crate::prelude::{KeyExpr, ZenohId}; +use crate::prelude::{KeyExpr, Value}; use crate::time::{new_reception_timestamp, Timestamp}; use crate::Priority; -use crate::Value; #[zenoh_macros::unstable] use serde::Serialize; use std::{ convert::{TryFrom, TryInto}, fmt, }; +use zenoh_protocol::core::EntityGlobalId; use zenoh_protocol::{core::CongestionControl, network::push::ext::QoSType}; pub type SourceSn = u64; @@ -52,7 +52,7 @@ pub(crate) struct DataInfo { pub kind: SampleKind, pub encoding: Option, pub timestamp: Option, - pub source_id: Option, + pub source_id: Option, pub source_sn: Option, pub qos: QoS, } @@ -61,16 +61,24 @@ pub(crate) struct DataInfo { #[zenoh_macros::unstable] #[derive(Debug, Clone)] pub struct SourceInfo { - /// The [`ZenohId`] of the zenoh instance that published the concerned [`Sample`]. - pub source_id: Option, + /// The [`EntityGlobalId`] of the zenoh entity that published the concerned [`Sample`]. + pub source_id: Option, /// The sequence number of the [`Sample`] from the source. pub source_sn: Option, } #[test] #[cfg(feature = "unstable")] +#[cfg(not(all(target_os = "macos", target_arch = "aarch64")))] fn source_info_stack_size() { - assert_eq!(std::mem::size_of::(), 16 * 2); + assert_eq!(std::mem::size_of::(), 40); +} + +#[test] +#[cfg(feature = "unstable")] +#[cfg(all(target_os = "macos", target_arch = "aarch64"))] +fn source_info_stack_size() { + assert_eq!(std::mem::size_of::(), 48); } #[zenoh_macros::unstable] diff --git a/zenoh/src/session.rs b/zenoh/src/session.rs index 87c416c209..861acf71de 100644 --- a/zenoh/src/session.rs +++ b/zenoh/src/session.rs @@ -48,7 +48,7 @@ use std::convert::TryFrom; use std::convert::TryInto; use std::fmt; use std::ops::Deref; -use std::sync::atomic::{AtomicU16, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicU16, Ordering}; use std::sync::Arc; use std::sync::RwLock; use std::time::Duration; @@ -57,6 +57,8 @@ use zenoh_buffers::ZBuf; use zenoh_collections::SingleOrVec; use zenoh_config::unwrap_or_default; use zenoh_core::{zconfigurable, zread, Resolve, ResolveClosure, ResolveFuture, SyncResolve}; +#[cfg(feature = "unstable")] +use zenoh_protocol::network::declare::SubscriberId; use zenoh_protocol::network::AtomicRequestId; use zenoh_protocol::network::RequestId; use zenoh_protocol::zenoh::reply::ReplyBody; @@ -97,9 +99,10 @@ pub(crate) struct SessionState { pub(crate) primitives: Option>, // @TODO replace with MaybeUninit ?? pub(crate) expr_id_counter: AtomicExprId, // @TODO: manage rollover and uniqueness pub(crate) qid_counter: AtomicRequestId, - pub(crate) decl_id_counter: AtomicUsize, pub(crate) local_resources: HashMap, pub(crate) remote_resources: HashMap, + #[cfg(feature = "unstable")] + pub(crate) remote_subscribers: HashMap>, //pub(crate) publications: Vec, pub(crate) subscribers: HashMap>, pub(crate) queryables: HashMap>, @@ -121,9 +124,10 @@ impl SessionState { primitives: None, expr_id_counter: AtomicExprId::new(1), // Note: start at 1 because 0 is reserved for NO_RESOURCE qid_counter: AtomicRequestId::new(0), - decl_id_counter: AtomicUsize::new(0), local_resources: HashMap::new(), remote_resources: HashMap::new(), + #[cfg(feature = "unstable")] + remote_subscribers: HashMap::new(), //publications: Vec::new(), subscribers: HashMap::new(), queryables: HashMap::new(), @@ -967,19 +971,20 @@ impl Session { ) -> ZResult> { let mut state = zwrite!(self.state); log::trace!("subscribe({:?})", key_expr); - let id = state.decl_id_counter.fetch_add(1, Ordering::SeqCst); + let id = self.runtime.next_id(); let key_expr = match scope { Some(scope) => scope / key_expr, None => key_expr.clone(), }; - let sub_state = Arc::new(SubscriberState { + let mut sub_state = SubscriberState { id, + remote_id: id, key_expr: key_expr.clone().into_owned(), scope: scope.clone().map(|e| e.into_owned()), origin, callback, - }); + }; #[cfg(not(feature = "unstable"))] let declared_sub = origin != Locality::SessionLocal; @@ -989,29 +994,39 @@ impl Session { .as_str() .starts_with(crate::liveliness::PREFIX_LIVELINESS); - let declared_sub = declared_sub - .then(|| { - match state - .aggregated_subscribers // TODO: can this be an OwnedKeyExpr? - .iter() - .find(|s| s.includes( &key_expr)) - { - Some(join_sub) => { - let joined_sub = state.subscribers.values().any(|s| { - s.origin != Locality::SessionLocal && join_sub.includes(&s.key_expr) - }); - (!joined_sub).then(|| join_sub.clone().into()) - } - None => { - let twin_sub = state - .subscribers - .values() - .any(|s| s.origin != Locality::SessionLocal && s.key_expr == key_expr); - (!twin_sub).then(|| key_expr.clone()) + let declared_sub = + declared_sub + .then(|| { + match state + .aggregated_subscribers + .iter() + .find(|s| s.includes(&key_expr)) + { + Some(join_sub) => { + if let Some(joined_sub) = state.subscribers.values().find(|s| { + s.origin != Locality::SessionLocal && join_sub.includes(&s.key_expr) + }) { + sub_state.remote_id = joined_sub.remote_id; + None + } else { + Some(join_sub.clone().into()) + } + } + None => { + if let Some(twin_sub) = state.subscribers.values().find(|s| { + s.origin != Locality::SessionLocal && s.key_expr == key_expr + }) { + sub_state.remote_id = twin_sub.remote_id; + None + } else { + Some(key_expr.clone()) + } + } } - } - }) - .flatten(); + }) + .flatten(); + + let sub_state = Arc::new(sub_state); state.subscribers.insert(sub_state.id, sub_state.clone()); for res in state @@ -1064,7 +1079,7 @@ impl Session { ext_tstamp: None, ext_nodeid: declare::ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: id as u32, + id, wire_expr: key_expr.to_wire(self).to_owned(), ext_info: *info, }), @@ -1080,7 +1095,7 @@ impl Session { Ok(sub_state) } - pub(crate) fn unsubscribe(&self, sid: usize) -> ZResult<()> { + pub(crate) fn unsubscribe(&self, sid: Id) -> ZResult<()> { let mut state = zwrite!(self.state); if let Some(sub_state) = state.subscribers.remove(&sid) { trace!("unsubscribe({:?})", sub_state); @@ -1110,65 +1125,28 @@ impl Session { if send_forget { // Note: there might be several Subscribers on the same KeyExpr. // Before calling forget_subscriber(key_expr), check if this was the last one. - let key_expr = &sub_state.key_expr; - match state - .aggregated_subscribers - .iter() - .find(|s| s.includes(key_expr)) - { - Some(join_sub) => { - let joined_sub = state.subscribers.values().any(|s| { - s.origin != Locality::SessionLocal && join_sub.includes(&s.key_expr) - }); - if !joined_sub { - let primitives = state.primitives.as_ref().unwrap().clone(); - let wire_expr = WireExpr::from(join_sub).to_owned(); - drop(state); - primitives.send_declare(Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { wire_expr }, - }), - }); - - #[cfg(feature = "unstable")] - { - let state = zread!(self.state); - self.update_status_down(&state, &sub_state.key_expr) - } - } - } - None => { - let twin_sub = state - .subscribers - .values() - .any(|s| s.origin != Locality::SessionLocal && s.key_expr == *key_expr); - if !twin_sub { - let primitives = state.primitives.as_ref().unwrap().clone(); - drop(state); - primitives.send_declare(Declare { - ext_qos: ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { - wire_expr: key_expr.to_wire(self).to_owned(), - }, - }), - }); - - #[cfg(feature = "unstable")] - { - let state = zread!(self.state); - self.update_status_down(&state, &sub_state.key_expr) - } - } + if !state.subscribers.values().any(|s| { + s.origin != Locality::SessionLocal && s.remote_id == sub_state.remote_id + }) { + let primitives = state.primitives.as_ref().unwrap().clone(); + drop(state); + primitives.send_declare(Declare { + ext_qos: declare::ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: declare::ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { + id: sub_state.remote_id, + ext_wire_expr: WireExprType { + wire_expr: WireExpr::empty(), + }, + }), + }); + #[cfg(feature = "unstable")] + { + let state = zread!(self.state); + self.update_status_down(&state, &sub_state.key_expr) } - }; + } } Ok(()) } else { @@ -1185,7 +1163,7 @@ impl Session { ) -> ZResult> { let mut state = zwrite!(self.state); log::trace!("queryable({:?})", key_expr); - let id = state.decl_id_counter.fetch_add(1, Ordering::SeqCst); + let id = self.runtime.next_id(); let qable_state = Arc::new(QueryableState { id, key_expr: key_expr.to_owned(), @@ -1193,158 +1171,48 @@ impl Session { origin, callback, }); - #[cfg(feature = "complete_n")] - { - state.queryables.insert(id, qable_state.clone()); - if origin != Locality::SessionLocal && complete { - let primitives = state.primitives.as_ref().unwrap().clone(); - let complete = Session::complete_twin_qabls(&state, key_expr); - drop(state); - let qabl_info = QueryableInfo { - complete, - distance: 0, - }; - primitives.send_declare(Declare { - ext_qos: declare::ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: declare::ext::NodeIdType::DEFAULT, - body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: id as u32, - wire_expr: key_expr.to_owned(), - ext_info: qabl_info, - }), - }); - } - } - #[cfg(not(feature = "complete_n"))] - { - let twin_qabl = Session::twin_qabl(&state, key_expr); - let complete_twin_qabl = twin_qabl && Session::complete_twin_qabl(&state, key_expr); - - state.queryables.insert(id, qable_state.clone()); + state.queryables.insert(id, qable_state.clone()); - if origin != Locality::SessionLocal && (!twin_qabl || (!complete_twin_qabl && complete)) - { - let primitives = state.primitives.as_ref().unwrap().clone(); - let complete = u8::from(!complete_twin_qabl && complete); - drop(state); - let qabl_info = QueryableInfo { - complete, - distance: 0, - }; - primitives.send_declare(Declare { - ext_qos: declare::ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: declare::ext::NodeIdType::DEFAULT, - body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: id as u32, - wire_expr: key_expr.to_owned(), - ext_info: qabl_info, - }), - }); - } + if origin != Locality::SessionLocal { + let primitives = state.primitives.as_ref().unwrap().clone(); + drop(state); + let qabl_info = QueryableInfo { + complete: if complete { 1 } else { 0 }, + distance: 0, + }; + primitives.send_declare(Declare { + ext_qos: declare::ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: declare::ext::NodeIdType::DEFAULT, + body: DeclareBody::DeclareQueryable(DeclareQueryable { + id, + wire_expr: key_expr.to_owned(), + ext_info: qabl_info, + }), + }); } Ok(qable_state) } - pub(crate) fn twin_qabl(state: &SessionState, key: &WireExpr) -> bool { - state.queryables.values().any(|q| { - q.origin != Locality::SessionLocal - && state.local_wireexpr_to_expr(&q.key_expr).unwrap() - == state.local_wireexpr_to_expr(key).unwrap() - }) - } - - #[cfg(not(feature = "complete_n"))] - pub(crate) fn complete_twin_qabl(state: &SessionState, key: &WireExpr) -> bool { - state.queryables.values().any(|q| { - q.origin != Locality::SessionLocal - && q.complete - && state.local_wireexpr_to_expr(&q.key_expr).unwrap() - == state.local_wireexpr_to_expr(key).unwrap() - }) - } - - #[cfg(feature = "complete_n")] - pub(crate) fn complete_twin_qabls(state: &SessionState, key: &WireExpr) -> u8 { - state - .queryables - .values() - .filter(|q| { - q.origin != Locality::SessionLocal - && q.complete - && state.local_wireexpr_to_expr(&q.key_expr).unwrap() - == state.local_wireexpr_to_expr(key).unwrap() - }) - .count() as u8 - } - - pub(crate) fn close_queryable(&self, qid: usize) -> ZResult<()> { + pub(crate) fn close_queryable(&self, qid: Id) -> ZResult<()> { let mut state = zwrite!(self.state); if let Some(qable_state) = state.queryables.remove(&qid) { trace!("close_queryable({:?})", qable_state); if qable_state.origin != Locality::SessionLocal { let primitives = state.primitives.as_ref().unwrap().clone(); - if Session::twin_qabl(&state, &qable_state.key_expr) { - // There still exist Queryables on the same KeyExpr. - if qable_state.complete { - #[cfg(feature = "complete_n")] - { - let complete = - Session::complete_twin_qabls(&state, &qable_state.key_expr); - drop(state); - let qabl_info = QueryableInfo { - complete, - distance: 0, - }; - primitives.send_declare(Declare { - ext_qos: declare::ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: declare::ext::NodeIdType::DEFAULT, - body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - wire_expr: qable_state.key_expr.clone(), - ext_info: qabl_info, - }), - }); - } - #[cfg(not(feature = "complete_n"))] - { - if !Session::complete_twin_qabl(&state, &qable_state.key_expr) { - drop(state); - let qabl_info = QueryableInfo { - complete: 0, - distance: 0, - }; - primitives.send_declare(Declare { - ext_qos: declare::ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: declare::ext::NodeIdType::DEFAULT, - body: DeclareBody::DeclareQueryable(DeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - wire_expr: qable_state.key_expr.clone(), - ext_info: qabl_info, - }), - }); - } - } - } - } else { - // There are no more Queryables on the same KeyExpr. - drop(state); - primitives.send_declare(Declare { - ext_qos: declare::ext::QoSType::DECLARE, - ext_tstamp: None, - ext_nodeid: declare::ext::NodeIdType::DEFAULT, - body: DeclareBody::UndeclareQueryable(UndeclareQueryable { - id: 0, // @TODO use proper QueryableId (#703) - ext_wire_expr: WireExprType { - wire_expr: qable_state.key_expr.clone(), - }, - }), - }); - } + drop(state); + primitives.send_declare(Declare { + ext_qos: declare::ext::QoSType::DECLARE, + ext_tstamp: None, + ext_nodeid: declare::ext::NodeIdType::DEFAULT, + body: DeclareBody::UndeclareQueryable(UndeclareQueryable { + id: qable_state.id, + ext_wire_expr: WireExprType { + wire_expr: qable_state.key_expr.clone(), + }, + }), + }); } Ok(()) } else { @@ -1359,7 +1227,7 @@ impl Session { ) -> ZResult> { let mut state = zwrite!(self.state); log::trace!("declare_liveliness({:?})", key_expr); - let id = state.decl_id_counter.fetch_add(1, Ordering::SeqCst); + let id = self.runtime.next_id(); let key_expr = KeyExpr::from(*crate::liveliness::KE_PREFIX_LIVELINESS / key_expr); let tok_state = Arc::new(LivelinessTokenState { id, @@ -1374,7 +1242,7 @@ impl Session { ext_tstamp: None, ext_nodeid: declare::ext::NodeIdType::DEFAULT, body: DeclareBody::DeclareSubscriber(DeclareSubscriber { - id: id as u32, + id, wire_expr: key_expr.to_wire(self).to_owned(), ext_info: SubscriberInfo::DEFAULT, }), @@ -1383,7 +1251,7 @@ impl Session { } #[zenoh_macros::unstable] - pub(crate) fn undeclare_liveliness(&self, tid: usize) -> ZResult<()> { + pub(crate) fn undeclare_liveliness(&self, tid: Id) -> ZResult<()> { let mut state = zwrite!(self.state); if let Some(tok_state) = state.tokens.remove(&tid) { trace!("undeclare_liveliness({:?})", tok_state); @@ -1398,10 +1266,8 @@ impl Session { ext_tstamp: None, ext_nodeid: ext::NodeIdType::DEFAULT, body: DeclareBody::UndeclareSubscriber(UndeclareSubscriber { - id: 0, // @TODO use proper SubscriberId (#703) - ext_wire_expr: WireExprType { - wire_expr: key_expr.to_wire(self).to_owned(), - }, + id: tok_state.id, + ext_wire_expr: WireExprType::null(), }), }); } @@ -1418,8 +1284,7 @@ impl Session { callback: Callback<'static, MatchingStatus>, ) -> ZResult> { let mut state = zwrite!(self.state); - - let id = state.decl_id_counter.fetch_add(1, Ordering::SeqCst); + let id = self.runtime.next_id(); log::trace!("matches_listener({:?}) => {id}", publisher.key_expr); let listener_state = Arc::new(MatchingListenerState { id, @@ -1554,7 +1419,7 @@ impl Session { } #[zenoh_macros::unstable] - pub(crate) fn undeclare_matches_listener_inner(&self, sid: usize) -> ZResult<()> { + pub(crate) fn undeclare_matches_listener_inner(&self, sid: Id) -> ZResult<()> { let mut state = zwrite!(self.state); if let Some(state) = state.matching_listeners.remove(&sid) { trace!("undeclare_matches_listener_inner({:?})", state); @@ -1856,15 +1721,15 @@ impl Session { body: Option, #[cfg(feature = "unstable")] attachment: Option, ) { - let (primitives, key_expr, callbacks) = { + let (primitives, key_expr, queryables) = { let state = zread!(self.state); match state.wireexpr_to_keyexpr(key_expr, local) { Ok(key_expr) => { - let callbacks = state + let queryables = state .queryables - .values() + .iter() .filter( - |queryable| + |(_, queryable)| (queryable.origin == Locality::Any || (local == (queryable.origin == Locality::SessionLocal))) && @@ -1881,12 +1746,12 @@ impl Session { } } ) - .map(|qable| qable.callback.clone()) - .collect::>>(); + .map(|(id, qable)| (*id, qable.callback.clone())) + .collect::)>>(); ( state.primitives.as_ref().unwrap().clone(), key_expr.into_owned(), - callbacks, + queryables, ) } Err(err) => { @@ -1898,29 +1763,30 @@ impl Session { let parameters = parameters.to_owned(); - let zid = self.runtime.zid(); // @TODO build/use prebuilt specific zid + let zid = self.runtime.zid(); - let query = Query { - inner: Arc::new(QueryInner { - key_expr, - parameters, - value: body.map(|b| Value { - payload: b.payload.into(), - encoding: b.encoding.into(), - }), - qid, - zid, - primitives: if local { - Arc::new(self.clone()) - } else { - primitives - }, - #[cfg(feature = "unstable")] - attachment, + let query_inner = Arc::new(QueryInner { + key_expr, + parameters, + value: body.map(|b| Value { + payload: b.payload.into(), + encoding: b.encoding.into(), }), - }; - for callback in callbacks.iter() { - callback(query.clone()); + qid, + zid, + primitives: if local { + Arc::new(self.clone()) + } else { + primitives + }, + #[cfg(feature = "unstable")] + attachment, + }); + for (eid, callback) in queryables { + callback(Query { + inner: query_inner.clone(), + eid, + }); } } } @@ -2111,9 +1977,13 @@ impl Primitives for Session { trace!("recv DeclareSubscriber {} {:?}", m.id, m.wire_expr); #[cfg(feature = "unstable")] { - let state = zread!(self.state); - match state.wireexpr_to_keyexpr(&m.wire_expr, false) { + let mut state = zwrite!(self.state); + match state + .wireexpr_to_keyexpr(&m.wire_expr, false) + .map(|e| e.into_owned()) + { Ok(expr) => { + state.remote_subscribers.insert(m.id, expr.clone()); self.update_status_up(&state, &expr); if expr @@ -2141,33 +2011,30 @@ impl Primitives for Session { trace!("recv UndeclareSubscriber {:?}", m.id); #[cfg(feature = "unstable")] { - let state = zread!(self.state); - match state.wireexpr_to_keyexpr(&m.ext_wire_expr.wire_expr, false) { - Ok(expr) => { - self.update_status_down(&state, &expr); + let mut state = zwrite!(self.state); + if let Some(expr) = state.remote_subscribers.remove(&m.id) { + self.update_status_down(&state, &expr); - if expr - .as_str() - .starts_with(crate::liveliness::PREFIX_LIVELINESS) - { - drop(state); - let data_info = DataInfo { - kind: SampleKind::Delete, - ..Default::default() - }; - self.handle_data( - false, - &m.ext_wire_expr.wire_expr, - Some(data_info), - ZBuf::default(), - #[cfg(feature = "unstable")] - None, - ); - } - } - Err(err) => { - log::error!("Received Forget Subscriber for unkown key_expr: {}", err) + if expr + .as_str() + .starts_with(crate::liveliness::PREFIX_LIVELINESS) + { + drop(state); + let data_info = DataInfo { + kind: SampleKind::Delete, + ..Default::default() + }; + self.handle_data( + false, + &m.ext_wire_expr.wire_expr, + Some(data_info), + ZBuf::default(), + #[cfg(feature = "unstable")] + None, + ); } + } else { + log::error!("Received Undeclare Subscriber for unkown id: {}", m.id); } } } @@ -2194,7 +2061,7 @@ impl Primitives for Session { encoding: Some(m.encoding.into()), timestamp: m.timestamp, qos: QoS::from(msg.ext_qos), - source_id: m.ext_sinfo.as_ref().map(|i| i.zid), + source_id: m.ext_sinfo.as_ref().map(|i| i.id.clone()), source_sn: m.ext_sinfo.as_ref().map(|i| i.sn as u64), }; self.handle_data( @@ -2212,7 +2079,7 @@ impl Primitives for Session { encoding: None, timestamp: m.timestamp, qos: QoS::from(msg.ext_qos), - source_id: m.ext_sinfo.as_ref().map(|i| i.zid), + source_id: m.ext_sinfo.as_ref().map(|i| i.id.clone()), source_sn: m.ext_sinfo.as_ref().map(|i| i.sn as u64), }; self.handle_data( @@ -2272,7 +2139,7 @@ impl Primitives for Session { }, }; let replier_id = match e.ext_sinfo { - Some(info) => info.zid, + Some(info) => info.id.zid, None => ZenohId::rand(), }; let new_reply = Reply { @@ -2366,7 +2233,7 @@ impl Primitives for Session { encoding: Some(encoding.into()), timestamp, qos: QoS::from(msg.ext_qos), - source_id: ext_sinfo.as_ref().map(|i| i.zid), + source_id: ext_sinfo.as_ref().map(|i| i.id.clone()), source_sn: ext_sinfo.as_ref().map(|i| i.sn as u64), }, #[cfg(feature = "unstable")] @@ -2384,7 +2251,7 @@ impl Primitives for Session { encoding: None, timestamp, qos: QoS::from(msg.ext_qos), - source_id: ext_sinfo.as_ref().map(|i| i.zid), + source_id: ext_sinfo.as_ref().map(|i| i.id.clone()), source_sn: ext_sinfo.as_ref().map(|i| i.sn as u64), }, #[cfg(feature = "unstable")] diff --git a/zenoh/src/subscriber.rs b/zenoh/src/subscriber.rs index c707218017..e276d0c6d0 100644 --- a/zenoh/src/subscriber.rs +++ b/zenoh/src/subscriber.rs @@ -25,6 +25,8 @@ use std::future::Ready; use std::ops::{Deref, DerefMut}; use std::sync::Arc; use zenoh_core::{AsyncResolve, Resolvable, Resolve, SyncResolve}; +#[cfg(feature = "unstable")] +use zenoh_protocol::core::EntityGlobalId; use zenoh_protocol::network::declare::{subscriber::ext::SubscriberInfo, Mode}; /// The kind of reliability. @@ -32,6 +34,7 @@ pub use zenoh_protocol::core::Reliability; pub(crate) struct SubscriberState { pub(crate) id: Id, + pub(crate) remote_id: Id, pub(crate) key_expr: KeyExpr<'static>, pub(crate) scope: Option>, pub(crate) origin: Locality, @@ -741,6 +744,29 @@ impl<'a, Receiver> PullSubscriber<'a, Receiver> { } impl<'a, Receiver> Subscriber<'a, Receiver> { + /// Returns the [`EntityGlobalId`] of this Subscriber. + /// + /// # Examples + /// ``` + /// # async_std::task::block_on(async { + /// use zenoh::prelude::r#async::*; + /// + /// let session = zenoh::open(config::peer()).res().await.unwrap(); + /// let subscriber = session.declare_subscriber("key/expression") + /// .res() + /// .await + /// .unwrap(); + /// let subscriber_id = subscriber.id(); + /// # }) + /// ``` + #[zenoh_macros::unstable] + pub fn id(&self) -> EntityGlobalId { + EntityGlobalId { + zid: self.subscriber.session.zid(), + eid: self.subscriber.state.id, + } + } + /// Returns the [`KeyExpr`] this Subscriber subscribes to. pub fn key_expr(&self) -> &KeyExpr<'static> { &self.subscriber.state.key_expr