Namespace prefix support #1792
Namespace prefix support #1792
Conversation
|
PR missing one of the required labels: {'internal', 'bug', 'enhancement', 'dependencies', 'documentation', 'new feature', 'breaking-change'} |
|
Btw this needs to be tested also with rmw_zenoh, idea on how to do it? |
| }; | ||
|
|
||
| pub(crate) static KE_UHLC: &keyexpr = ke!("uhlc"); | ||
| #[zenoh_macros::unstable] | ||
| kedefine!( | ||
| pub(crate) ke_liveliness: "@adv/${entity:*}/${zid:*}/${eid:*}/${meta:**}/@/${remaining:**}", | ||
| pub(crate) ke_liveliness: "${remaining:**}/@adv/${entity:*}/${zid:*}/${eid:*}/${meta:**}", |
There was a problem hiding this comment.
In the current form a keyexpr is transformed from a/b/c to @foo/prefix/a/b/c.
Would it make sense to simply invert the order to a/b/c/@foo/prefix?
Would it make the whole namespace prefix logic simpler?
There was a problem hiding this comment.
I'm not sure I understand the comment, In this PR it is indeed supposed to be transformed into a/b/c/@foo/prefix
There was a problem hiding this comment.
Thanks for the clarification. I see the intended behaviour was introduced by #1780 and integrated in this PR. I was momentarily misled by this PR title.
Maybe @evshary have some ideas, otherwise I suppose we would need to create dedicated branches on zenoh-c and zenoh-cpp. |
DenisBiryukov91
force-pushed
the
namespace-as-dyn
branch
from
81fbc9e to
e002c6a
Compare
|
The implementation LGTM. However it should be tested with |
|
Sister PRs may be needed for bindings, as just changing the branch makes zenoh-c CI to fail: https://github.com/eclipse-zenoh/zenoh-c/actions/runs/13541109439/ |
I did run some tests on my machine:
and I got: current main 8 bytes payload, routed: this branch 8 bytes payload, routed, namespace is : so a few hundreds k/msgs/s are lost may need to re-run the test on a better machine, instead of the laptop |
|
Here results on a proper machine Throughputmain, p2p namespace-as-dyn, namespace = "test", p2p namespace-as-dyn, no namespace, p2p Latencymain, p2p namespace-as-dyn, namespace = "test", p2p namespace-as-dyn, no namespace, p2p |
Namespace prefix support
Supersedes #1772.
Includes #1780.
Related PRs to account for api changes:
eclipse-zenoh/zenoh-c#930
eclipse-zenoh/zenoh-cpp#417