From 3b9626d79af905d9b2db3b951fcfdb6859600065 Mon Sep 17 00:00:00 2001 From: Christien Rioux Date: Tue, 20 Aug 2024 22:29:02 +0000 Subject: [PATCH] implement issue #388: Change app-facing default to Sequencing::PreferOrdered --- veilid-core/src/routing_table/bucket_entry.rs | 16 ++++------------ .../src/routing_table/route_spec_store/mod.rs | 5 +++-- .../tasks/private_route_management.rs | 4 ++-- veilid-core/src/veilid_api/api.rs | 4 ++-- veilid-core/src/veilid_api/routing_context.rs | 6 +++--- .../example/integration_test/test_dht.dart | 3 +-- veilid-python/tests/test_dht.py | 4 +--- veilid-python/veilid/types.py | 2 +- 8 files changed, 17 insertions(+), 27 deletions(-) diff --git a/veilid-core/src/routing_table/bucket_entry.rs b/veilid-core/src/routing_table/bucket_entry.rs index ce00ca25..6360bfb4 100644 --- a/veilid-core/src/routing_table/bucket_entry.rs +++ b/veilid-core/src/routing_table/bucket_entry.rs @@ -268,17 +268,7 @@ impl BucketEntryInner { } // Lower latency to the front - if let Some(e1_latency) = &e1.peer_stats.latency { - if let Some(e2_latency) = &e2.peer_stats.latency { - e1_latency.average.cmp(&e2_latency.average) - } else { - std::cmp::Ordering::Less - } - } else if e2.peer_stats.latency.is_some() { - std::cmp::Ordering::Greater - } else { - std::cmp::Ordering::Equal - } + Self::cmp_fastest(e1, e2) } // Less is more reliable then older @@ -290,6 +280,7 @@ impl BucketEntryInner { } // Lower timestamp to the front, recent or no timestamp is at the end + // First check consecutive-ping reliability timestamp if let Some(e1_ts) = &e1.peer_stats.rpc_stats.first_consecutive_seen_ts { if let Some(e2_ts) = &e2.peer_stats.rpc_stats.first_consecutive_seen_ts { e1_ts.cmp(e2_ts) @@ -299,7 +290,8 @@ impl BucketEntryInner { } else if e2.peer_stats.rpc_stats.first_consecutive_seen_ts.is_some() { std::cmp::Ordering::Greater } else { - std::cmp::Ordering::Equal + // Then check 'since added to routing table' timestamp + e1.peer_stats.time_added.cmp(&e2.peer_stats.time_added) } } diff --git a/veilid-core/src/routing_table/route_spec_store/mod.rs b/veilid-core/src/routing_table/route_spec_store/mod.rs index 87fc944d..4fc4d510 100644 --- a/veilid-core/src/routing_table/route_spec_store/mod.rs +++ b/veilid-core/src/routing_table/route_spec_store/mod.rs @@ -362,7 +362,7 @@ impl RouteSpecStore { entry1: &Option>, entry2: &Option>| -> Ordering { - // Our own node is filtered out + // Our own node is filtered out, so it is safe to unwrap here let entry1 = entry1.as_ref().unwrap().clone(); let entry2 = entry2.as_ref().unwrap().clone(); let entry1_node_ids = entry1.with_inner(|e| e.node_ids()); @@ -398,6 +398,7 @@ impl RouteSpecStore { .signed_node_info(RoutingDomain::PublicInternet) .map(|sni| sni.has_sequencing_matched_dial_info(sequencing)) .unwrap_or(false); + // Reverse this comparison because ordered is preferable (less) e2_can_do_ordered.cmp(&e1_can_do_ordered) }) }); @@ -406,8 +407,8 @@ impl RouteSpecStore { } } + // apply stability preference // always prioritize reliable nodes, but sort by oldest or fastest - entry1.with_inner(|e1| { entry2.with_inner(|e2| match stability { Stability::LowLatency => BucketEntryInner::cmp_fastest_reliable(cur_ts, e1, e2), diff --git a/veilid-core/src/routing_table/tasks/private_route_management.rs b/veilid-core/src/routing_table/tasks/private_route_management.rs index f0cf4420..469a90cf 100644 --- a/veilid-core/src/routing_table/tasks/private_route_management.rs +++ b/veilid-core/src/routing_table/tasks/private_route_management.rs @@ -213,8 +213,8 @@ impl RoutingTable { // These will be used by test_remote_route as well match rss.allocate_route( &VALID_CRYPTO_KINDS, - Stability::default(), - Sequencing::EnsureOrdered, + Stability::Reliable, + Sequencing::PreferOrdered, default_route_hop_count, DirectionSet::all(), &[], diff --git a/veilid-core/src/veilid_api/api.rs b/veilid-core/src/veilid_api/api.rs index 3855ef96..54f8b6e4 100644 --- a/veilid-core/src/veilid_api/api.rs +++ b/veilid-core/src/veilid_api/api.rs @@ -253,7 +253,7 @@ impl VeilidAPI { // Private route allocation /// Allocate a new private route set with default cryptography and network options. - /// Default settings are for [Stability::Reliable] and [Sequencing::EnsureOrdered]. + /// Default settings are for [Stability::Reliable] and [Sequencing::PreferOrdered]. /// Returns a route id and a publishable 'blob' with the route encrypted with each crypto kind. /// Those nodes importing the blob will have their choice of which crypto kind to use. /// @@ -264,7 +264,7 @@ impl VeilidAPI { self.new_custom_private_route( &VALID_CRYPTO_KINDS, Stability::Reliable, - Sequencing::EnsureOrdered, + Sequencing::PreferOrdered, ) .await } diff --git a/veilid-core/src/veilid_api/routing_context.rs b/veilid-core/src/veilid_api/routing_context.rs index 93432415..0f21d2ae 100644 --- a/veilid-core/src/veilid_api/routing_context.rs +++ b/veilid-core/src/veilid_api/routing_context.rs @@ -56,7 +56,7 @@ impl RoutingContext { preferred_route: None, hop_count: c.network.rpc.default_route_hop_count as usize, stability: Stability::Reliable, - sequencing: Sequencing::EnsureOrdered, + sequencing: Sequencing::PreferOrdered, }), }), }) @@ -68,7 +68,7 @@ impl RoutingContext { /// Default values for hop count, stability and sequencing preferences are used. /// /// * Hop count default is dependent on config, but is set to 1 extra hop. - /// * Stability default is to choose 'low latency' routes, preferring them over long-term reliability. + /// * Stability default is to choose reliable routes, preferring them over low latency. /// * Sequencing default is to prefer ordered before unordered message delivery. /// /// To customize the safety selection in use, use [RoutingContext::with_safety()]. @@ -84,7 +84,7 @@ impl RoutingContext { preferred_route: None, hop_count: c.network.rpc.default_route_hop_count as usize, stability: Stability::Reliable, - sequencing: Sequencing::EnsureOrdered, + sequencing: Sequencing::PreferOrdered, })) } diff --git a/veilid-flutter/example/integration_test/test_dht.dart b/veilid-flutter/example/integration_test/test_dht.dart index 1da6cb6a..aea083bc 100644 --- a/veilid-flutter/example/integration_test/test_dht.dart +++ b/veilid-flutter/example/integration_test/test_dht.dart @@ -283,8 +283,7 @@ Future testWatchDHTValues(Stream updateStream) async { // as the watch's target final rcSet = await Veilid.instance.routingContext(); - final rcWatch = await Veilid.instance - .unsafeRoutingContext(sequencing: Sequencing.ensureOrdered); + final rcWatch = await Veilid.instance.unsafeRoutingContext(); try { // Make a DHT record var rec = await rcWatch.createDHTRecord(const DHTSchema.dflt(oCnt: 10)); diff --git a/veilid-python/tests/test_dht.py b/veilid-python/tests/test_dht.py index 175875b8..1436fa40 100644 --- a/veilid-python/tests/test_dht.py +++ b/veilid-python/tests/test_dht.py @@ -227,9 +227,7 @@ async def test_watch_dht_values(): # as the watch's target rcWatch = await api.new_routing_context() - rcSet = await (await api.new_routing_context()).with_safety( - veilid.SafetySelection.unsafe(veilid.Sequencing.ENSURE_ORDERED) - ) + rcSet = await (await api.new_routing_context()).with_safety(veilid.SafetySelection.unsafe()) async with rcWatch, rcSet: # Make a DHT record rec = await rcWatch.create_dht_record(veilid.DHTSchema.dflt(10)) diff --git a/veilid-python/veilid/types.py b/veilid-python/veilid/types.py index 56e52002..8ebad0fa 100644 --- a/veilid-python/veilid/types.py +++ b/veilid-python/veilid/types.py @@ -506,7 +506,7 @@ class SafetySelection: setattr(self, k, v) @classmethod - def unsafe(cls, sequencing: Sequencing) -> Self: + def unsafe(cls, sequencing: Sequencing = Sequencing.PREFER_ORDERED) -> Self: return cls(SafetySelectionKind.UNSAFE, sequencing=sequencing) @classmethod