Skip to content

Commit

Permalink
Fix bug with QueryTarget ALL_COMPLETE in clients and peers (#1358)
Browse files Browse the repository at this point in the history 
* Fix bug with QueryTarget ALL_COMPLETE in clients and peers

* Fix BEST_MATCHING queryable selection

* Properly fix Query targeting in non writer side filtering situations

* Improve fix

* Update zenoh/src/net/routing/hat/linkstate_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/p2p_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/router/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/client/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Remove non used ordered-float dependency

---------

Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Joseph Perez <[email protected]>
  • Loading branch information
@OlivierHecart @Mallets @wyfo
3 people authored Sep 6, 2024
1 parent db099c1 commit 6b7ec55
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 68 deletions.
10 changes: 0 additions & 10 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion zenoh/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ git-version = { workspace = true }
itertools = { workspace = true }
lazy_static = { workspace = true }
tracing = { workspace = true }
ordered-float = { workspace = true }
paste = { workspace = true }
petgraph = { workspace = true }
phf = { workspace = true }
Expand Down
9 changes: 4 additions & 5 deletions zenoh/src/net/routing/dispatcher/queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ fn compute_final_route(
TargetType::AllComplete => {
let mut route = HashMap::new();
for qabl in qabls.iter() {
if qabl.complete > 0
if qabl.info.map(|info| info.complete).unwrap_or(true)
&& tables
.hat_code
.egress_filter(tables, src_face, &qabl.direction.0, expr)
Expand All @@ -342,10 +342,9 @@ fn compute_final_route(
route
}
TargetType::BestMatching => {
if let Some(qabl) = qabls
.iter()
.find(|qabl| qabl.direction.0.id != src_face.id && qabl.complete > 0)
{
if let Some(qabl) = qabls.iter().find(|qabl| {
qabl.direction.0.id != src_face.id && qabl.info.is_some_and(|info| info.complete)
}) {
let mut route = HashMap::new();

let mut direction = qabl.direction.clone();
Expand Down
3 changes: 1 addition & 2 deletions zenoh/src/net/routing/dispatcher/resource.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ pub(crate) type Route = HashMap<usize, Direction>;
pub(crate) type QueryRoute = HashMap<usize, (Direction, RequestId)>;
pub(crate) struct QueryTargetQabl {
pub(crate) direction: Direction,
pub(crate) complete: u64,
pub(crate) distance: f64,
pub(crate) info: Option<QueryableInfoType>,
}
pub(crate) type QueryTargetQablSet = Vec<QueryTargetQabl>;

Expand Down
16 changes: 6 additions & 10 deletions zenoh/src/net/routing/hat/client/queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::{
sync::{atomic::Ordering, Arc},
};

use ordered_float::OrderedFloat;
use zenoh_protocol::{
core::{
key_expr::{
Expand Down Expand Up @@ -354,8 +353,7 @@ impl HatQueriesTrait for HatCode {
let key_expr = Resource::get_best_key(expr.prefix, expr.suffix, face.id);
route.push(QueryTargetQabl {
direction: (face.clone(), key_expr.to_owned(), NodeId::default()),
complete: 0,
distance: f64::MAX,
info: None,
});
}
}
Expand All @@ -375,17 +373,15 @@ impl HatQueriesTrait for HatCode {
if let Some(qabl_info) = context.qabl.as_ref() {
route.push(QueryTargetQabl {
direction: (context.face.clone(), key_expr.to_owned(), NodeId::default()),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: 0.5,
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: 1,
}),
});
}
}
}
route.sort_by_key(|qabl| OrderedFloat(qabl.distance));
route.sort_by_key(|qabl| qabl.info.map_or(u16::MAX, |i| i.distance));
Arc::new(route)
}

Expand Down
23 changes: 9 additions & 14 deletions zenoh/src/net/routing/hat/linkstate_peer/queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::{
sync::{atomic::Ordering, Arc},
};

use ordered_float::OrderedFloat;
use petgraph::graph::NodeIndex;
use zenoh_protocol::{
core::{
Expand Down Expand Up @@ -718,12 +717,10 @@ fn insert_target_for_qabls(
Resource::get_best_key(expr.prefix, expr.suffix, face.id);
route.push(QueryTargetQabl {
direction: (face.clone(), key_expr.to_owned(), source),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: net.distances[qabl_idx.index()],
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: net.distances[qabl_idx.index()] as u16,
}),
});
}
}
Expand Down Expand Up @@ -1000,18 +997,16 @@ impl HatQueriesTrait for HatCode {
key_expr.to_owned(),
NodeId::default(),
),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: 0.5,
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: 1,
}),
});
}
}
}
}
route.sort_by_key(|qabl| OrderedFloat(qabl.distance));
route.sort_by_key(|qabl| qabl.info.map_or(u16::MAX, |i| i.distance));
Arc::new(route)
}

Expand Down
19 changes: 7 additions & 12 deletions zenoh/src/net/routing/hat/p2p_peer/queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::{
sync::{atomic::Ordering, Arc},
};

use ordered_float::OrderedFloat;
use zenoh_protocol::{
core::{
key_expr::{
Expand Down Expand Up @@ -597,8 +596,7 @@ impl HatQueriesTrait for HatCode {
let key_expr = Resource::get_best_key(expr.prefix, expr.suffix, face.id);
route.push(QueryTargetQabl {
direction: (face.clone(), key_expr.to_owned(), NodeId::default()),
complete: 0,
distance: f64::MAX,
info: None,
});
}

Expand All @@ -613,8 +611,7 @@ impl HatQueriesTrait for HatCode {
let key_expr = Resource::get_best_key(expr.prefix, expr.suffix, face.id);
route.push(QueryTargetQabl {
direction: (face.clone(), key_expr.to_owned(), NodeId::default()),
complete: 0,
distance: 0.5,
info: None,
});
}
}
Expand All @@ -639,18 +636,16 @@ impl HatQueriesTrait for HatCode {
key_expr.to_owned(),
NodeId::default(),
),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: 0.5,
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: 1,
}),
});
}
}
}
}
route.sort_by_key(|qabl| OrderedFloat(qabl.distance));
route.sort_by_key(|qabl| qabl.info.map_or(u16::MAX, |i| i.distance));
Arc::new(route)
}

Expand Down
23 changes: 9 additions & 14 deletions zenoh/src/net/routing/hat/router/queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::{
sync::{atomic::Ordering, Arc},
};

use ordered_float::OrderedFloat;
use petgraph::graph::NodeIndex;
use zenoh_protocol::{
core::{
Expand Down Expand Up @@ -1102,12 +1101,10 @@ fn insert_target_for_qabls(
Resource::get_best_key(expr.prefix, expr.suffix, face.id);
route.push(QueryTargetQabl {
direction: (face.clone(), key_expr.to_owned(), source),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: net.distances[qabl_idx.index()],
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: net.distances[qabl_idx.index()] as u16,
}),
});
}
}
Expand Down Expand Up @@ -1482,19 +1479,17 @@ impl HatQueriesTrait for HatCode {
key_expr.to_owned(),
NodeId::default(),
),
complete: if complete {
qabl_info.complete as u64
} else {
0
},
distance: 0.5,
info: Some(QueryableInfoType {
complete: complete && qabl_info.complete,
distance: 1,
}),
});
}
}
}
}
}
route.sort_by_key(|qabl| OrderedFloat(qabl.distance));
route.sort_by_key(|qabl| qabl.info.map_or(u16::MAX, |i| i.distance));
Arc::new(route)
}

Expand Down

0 comments on commit 6b7ec55

Please sign in to comment.