Skip to content

Set higher default value for Zenoh wait_before_drop parameter - #841

Closed
caguero wants to merge 5 commits into
mainfrom
caguero-zenoh_wait_before_drop
Closed

Set higher default value for Zenoh wait_before_drop parameter#841
caguero wants to merge 5 commits into
mainfrom
caguero-zenoh_wait_before_drop

Conversation

@caguero

@caguero caguero commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator

🦟 Bug fix

This patch increases the Zenoh wait_before_drop timeout from 1 ms (Zenoh default) to
50 ms, and max_wait_before_drop_fragments from 50 ms to 250 ms. This
eliminates message loss for large messages in inter-process pub/sub without
measurable throughput impact.

wait_drop_sweep

Instructions to reproduce the benchmark

Save this diff into a file:

diff --git a/example/benchmark/plot_benchmark.py b/example/benchmark/plot_benchmark.py
index 5c83ded2..aed7e24e 100755
--- a/example/benchmark/plot_benchmark.py
+++ b/example/benchmark/plot_benchmark.py
@@ -69,10 +69,20 @@ _SIZE_LABELS = [
 # Per-configuration display style.  Order here determines legend order.
 # Wong colorblind-safe palette — distinguishable by hue, even in greyscale.
 STYLES: 'OrderedDict[str, dict]' = OrderedDict([
-    ('zenoh',
-     {'label': 'Zenoh',   'color': '#009E73'}),
+    ('zenoh_default',
+     {'label': 'Zenoh default (1ms)', 'color': '#999999'}),
+    ('zenoh_wait1ms',
+     {'label': 'Zenoh 1ms (explicit)', 'color': '#D55E00'}),
+    ('zenoh_wait5ms',
+     {'label': 'Zenoh 5ms',           'color': '#E69F00'}),
+    ('zenoh_wait10ms',
+     {'label': 'Zenoh 10ms',          'color': '#CC79A7'}),
+    ('zenoh_wait50ms',
+     {'label': 'Zenoh 50ms',          'color': '#009E73'}),
+    ('zenoh_wait100ms',
+     {'label': 'Zenoh 100ms',         'color': '#56B4E9'}),
     ('zeromq',
-     {'label': 'ZeroMQ',  'color': '#0072B2'}),
+     {'label': 'ZeroMQ',              'color': '#0072B2'}),
 ])
 
 
diff --git a/example/benchmark/run_benchmark.py b/example/benchmark/run_benchmark.py
index 647c55ae..84db65d8 100755
--- a/example/benchmark/run_benchmark.py
+++ b/example/benchmark/run_benchmark.py
@@ -73,13 +73,57 @@ import time
 #   name: identifier used in output filenames
 #   env: dict of env vars to SET (keys not present are removed from env)
 #   clear: list of env var names to REMOVE
+_DROP_KEY = ('transport/link/tx/queue/congestion_control/drop/'
+             'wait_before_drop')
+_FRAG_KEY = ('transport/link/tx/queue/congestion_control/drop/'
+             'max_wait_before_drop_fragments')
+
+def _zenoh_wait_config(wait_us, frag_ratio=5):
+    """Build a Zenoh config with a specific wait_before_drop value."""
+    frag_us = wait_us * frag_ratio
+    return {
+        'GZ_TRANSPORT_IMPLEMENTATION': 'zenoh',
+        'GZ_TRANSPORT_ZENOH_CONFIG_OVERRIDE':
+            f'{_DROP_KEY}={wait_us};{_FRAG_KEY}={frag_us}',
+    }
+
 CONFIGS = [
     {
-        'name': 'zenoh',
-        'description': 'Zenoh (default settings)',
+        'name': 'zenoh_default',
+        'description': 'Zenoh (default, wait_before_drop=1ms)',
         'env': {
             'GZ_TRANSPORT_IMPLEMENTATION': 'zenoh',
         },
+        'clear': ['GZ_TRANSPORT_ZENOH_CONFIG_OVERRIDE'],
+    },
+    {
+        'name': 'zenoh_wait1ms',
+        'description': 'Zenoh (wait_before_drop=1ms, explicit)',
+        'env': _zenoh_wait_config(1000),
+        'clear': [],
+    },
+    {
+        'name': 'zenoh_wait5ms',
+        'description': 'Zenoh (wait_before_drop=5ms)',
+        'env': _zenoh_wait_config(5000),
+        'clear': [],
+    },
+    {
+        'name': 'zenoh_wait10ms',
+        'description': 'Zenoh (wait_before_drop=10ms)',
+        'env': _zenoh_wait_config(10000),
+        'clear': [],
+    },
+    {
+        'name': 'zenoh_wait50ms',
+        'description': 'Zenoh (wait_before_drop=50ms)',
+        'env': _zenoh_wait_config(50000),
+        'clear': [],
+    },
+    {
+        'name': 'zenoh_wait100ms',
+        'description': 'Zenoh (wait_before_drop=100ms)',
+        'env': _zenoh_wait_config(100000),
         'clear': [],
     },
     {
@@ -88,7 +132,7 @@ CONFIGS = [
         'env': {
             'GZ_TRANSPORT_IMPLEMENTATION': 'zeromq',
         },
-        'clear': [],
+        'clear': ['GZ_TRANSPORT_ZENOH_CONFIG_OVERRIDE'],
     },
 ]

And then, apply the path and run the benchmark:

cd ~/rotary_ws/src/gz-transport
git apply /tmp/sweep_wait_before_drop.patch
source ~/rotary_ws/install/setup.bash
cd example/benchmark
python3 run_benchmark.py -b build/bench -i 500 -w 10 -t --plot

Summary

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Remove this if GenAI was not used.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Backports: If this is a backport, please use Rebase and Merge instead.

Signed-off-by: Carlos Agüero <caguero@osrfoundation.org>
@azeey

azeey commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Nice! Could you include instructions for the benchmarking you did to discover this so that I or others can reproduce it?

@caguero

caguero commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator Author

Nice! Could you include instructions for the benchmarking you did to discover this so that I or others can reproduce it?

I've updated the description with the instructions.

Comment thread src/NodeSharedPrivate.hh
// the default 1 ms wait_before_drop causes 50-90% loss for
// messages in the 125 KB–4 MB range, while 50 ms eliminates
// loss entirely without measurable throughput impact.
// These can be overridden via ZENOH_CONFIG or

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be overridden via ZENOH_CONFIG

Is this correct? the ZENOH configurations seems to be loaded before these lines.

https://github.com/gazebosim/gz-transport/pull/841/changes#diff-652bc237f682bc3650223ea89ba77865c3569765a9943a546f53f655443c9e2fR100

@azeey azeey linked an issue May 26, 2026 that may be closed by this pull request
6 tasks
@caguero

caguero commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: we'll rely on Zenoh's default congestion-control parameters instead of overriding wait_before_drop. The SHM work continues in #842, now retargeted to main.

@caguero caguero closed this Jul 18, 2026
@github-project-automation github-project-automation Bot moved this from Inbox to Done in Core development Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Zenoh outstanding tasks

3 participants