Commit 7d5513012b4e added stopwatch_start()/stopwatch_stop() calls
around every engine node recompute and change handler invocation.
With ~320 engine stopwatches, each iteration incurs hundreds of
heap allocations, mutex operations, and syscalls (clock_gettime
and write on a pipe fd). This causes significant CPU increase in
scaled environments.
Guard the stopwatch_start()/stopwatch_stop() calls behind a new
per-node flag that defaults to false. Add two appctl commands,
"inc-engine/enable-stopwatch" and "inc-engine/disable-stopwatch",
to toggle the flag at runtime. Both commands accept an optional
node name argument for per-node granularity; without an argument
all nodes are affected. The one-time stopwatch_create() calls
in engine_init() and engine_add_input_impl() are left unchanged
because they have zero per-iteration cost.
Fixes: 7d5513012b4e ("inc-proc-eng: Build stopwatches into every incremental
node.")
Reported-at: https://redhat.atlassian.net/browse/FDP-4170
Assisted-by: Claude Opus 4.6, Claude Code
Signed-off-by: Dumitru Ceara <[email protected]>
---
controller/ovn-controller.8.xml | 20 +++++++
lib/inc-proc-eng.c | 60 +++++++++++++++++++--
lib/inc-proc-eng.h | 3 ++
northd/ovn-northd.8.xml | 20 +++++++
tests/ovn-northd.at | 96 +++++++++++++++++++++++++++++++++
5 files changed, 195 insertions(+), 4 deletions(-)
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index dc4d106e59..e355046901 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -896,6 +896,26 @@
<dd>
Reset <code>ovn-controller</code> engine counters.
</dd>
+
+ <dt><code>inc-engine/enable-stopwatch</code> [<var>node</var>]</dt>
+ <dd>
+ Enables the per-node and per-handler stopwatches in the
+ incremental processing engine. If <var>node</var> is specified,
+ only that node's stopwatches are enabled; otherwise all nodes
+ are affected. While enabled, every engine node recompute and
+ change handler invocation is timed and the results can be
+ viewed with the <code>stopwatch/show</code> command.
+ Stopwatches are disabled by default because they add measurable
+ CPU overhead.
+ </dd>
+
+ <dt><code>inc-engine/disable-stopwatch</code> [<var>node</var>]</dt>
+ <dd>
+ Disables the per-node and per-handler stopwatches in the
+ incremental processing engine. If <var>node</var> is specified,
+ only that node's stopwatches are disabled; otherwise all nodes
+ are affected. This is the default state.
+ </dd>
</dl>
</p>
diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index a4b6c8cde3..bcb0848c3b 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -220,6 +220,46 @@ engine_list_stopwatch_cmd(struct unixctl_conn *conn, int
argc OVS_UNUSED,
ds_destroy(&output_str);
}
+static void
+engine_set_stopwatch_cmd(struct unixctl_conn *conn, int argc,
+ const char *argv[], bool enabled)
+{
+ const char *node_name = argc > 1 ? argv[1] : NULL;
+
+ struct engine_node *node;
+ bool found = false;
+ VECTOR_FOR_EACH (&engine_nodes, node) {
+ if (node_name && strcmp(node->name, node_name)) {
+ continue;
+ }
+ node->stopwatch_enabled = enabled;
+ found = true;
+ if (node_name) {
+ break;
+ }
+ }
+
+ if (node_name && !found) {
+ unixctl_command_reply_error(conn, "node not found");
+ return;
+ }
+ unixctl_command_reply(conn, NULL);
+}
+
+static void
+engine_enable_stopwatches_cmd(struct unixctl_conn *conn, int argc,
+ const char *argv[], void *arg OVS_UNUSED)
+{
+ engine_set_stopwatch_cmd(conn, argc, argv, true);
+}
+
+static void
+engine_disable_stopwatches_cmd(struct unixctl_conn *conn, int argc,
+ const char *argv[], void *arg OVS_UNUSED)
+{
+ engine_set_stopwatch_cmd(conn, argc, argv, false);
+}
+
static void
engine_get_compute_failure_info(struct engine_node *node)
{
@@ -256,6 +296,10 @@ engine_init(struct engine_node *node, struct engine_arg
*arg)
engine_set_log_timeout_cmd, NULL);
unixctl_command_register("inc-engine/list-stopwatches", "", 0, 1,
engine_list_stopwatch_cmd, NULL);
+ unixctl_command_register("inc-engine/enable-stopwatch", "[node]", 0, 1,
+ engine_enable_stopwatches_cmd, NULL);
+ unixctl_command_register("inc-engine/disable-stopwatch", "[node]", 0, 1,
+ engine_disable_stopwatches_cmd, NULL);
}
void
@@ -455,9 +499,13 @@ static enum engine_node_state
run_recompute_callback(struct engine_node *node)
{
enum engine_node_state ret;
- stopwatch_start(node->name, time_msec());
+ if (node->stopwatch_enabled) {
+ stopwatch_start(node->name, time_msec());
+ }
ret = node->run(node, node->data);
- stopwatch_stop(node->name, time_msec());
+ if (node->stopwatch_enabled) {
+ stopwatch_stop(node->name, time_msec());
+ }
return ret;
}
@@ -465,9 +513,13 @@ static enum engine_input_handler_result
run_change_handler(struct engine_node *node, struct engine_node_input *input)
{
enum engine_input_handler_result ret;
- stopwatch_start(input->change_handler_name, time_msec());
+ if (node->stopwatch_enabled) {
+ stopwatch_start(input->change_handler_name, time_msec());
+ }
ret = input->change_handler(node, node->data);
- stopwatch_stop(input->change_handler_name, time_msec());
+ if (node->stopwatch_enabled) {
+ stopwatch_stop(input->change_handler_name, time_msec());
+ }
return ret;
}
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 1cb2466b23..ece33e1beb 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -282,6 +282,9 @@ struct engine_node {
/* Indication if the node writes to SB DB. */
bool sb_write;
+
+ /* Whether stopwatches are enabled for this node. */
+ bool stopwatch_enabled;
};
/* Initialize the data for the engine nodes. It calls each node's
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index a8db49f659..c2670601a7 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -257,6 +257,26 @@
node are listed.
</dd>
+ <dt><code>inc-engine/enable-stopwatch</code> [<var>node</var>]</dt>
+ <dd>
+ Enables the per-node and per-handler stopwatches in the
+ incremental processing engine. If <var>node</var> is specified,
+ only that node's stopwatches are enabled; otherwise all nodes
+ are affected. While enabled, every engine node recompute and
+ change handler invocation is timed and the results can be
+ viewed with the <code>stopwatch/show</code> command.
+ Stopwatches are disabled by default because they add measurable
+ CPU overhead.
+ </dd>
+
+ <dt><code>inc-engine/disable-stopwatch</code> [<var>node</var>]</dt>
+ <dd>
+ Disables the per-node and per-handler stopwatches in the
+ incremental processing engine. If <var>node</var> is specified,
+ only that node's stopwatches are disabled; otherwise all nodes
+ are affected. This is the default state.
+ </dd>
+
</dl>
</p>
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 484642b7a6..c58f731e90 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -21760,3 +21760,99 @@ ct_next(ct_state=new|trk) {
OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ovn-northd - engine stopwatches enable/disable])
+AT_KEYWORDS([ovn])
+ovn_start
+
+dnl By default, engine stopwatches are disabled. Trigger a recompute to
+dnl ensure the engine runs, then verify that no samples were collected.
+check as northd ovn-appctl -t ovn-northd inc-engine/recompute
+check ovn-nbctl --wait=sb sync
+AT_CHECK([as northd ovn-appctl -t ovn-northd stopwatch/show northd], [0], [dnl
+Statistics for 'northd'
+ Total samples: 0
+ Maximum: 0 msec
+ Minimum: 0 msec
+ 95th percentile: 0.000000 msec
+ Short term average: 0.000000 msec
+ Long term average: 0.000000 msec
+])
+
+dnl Enable stopwatches only for the "northd" node.
+check as northd ovn-appctl -t ovn-northd inc-engine/enable-stopwatch northd
+check as northd ovn-appctl -t ovn-northd inc-engine/recompute
+check ovn-nbctl --wait=sb sync
+
+dnl The "northd" stopwatch should have collected samples.
+OVS_WAIT_UNTIL([
+ stats=$(as northd ovn-appctl -t ovn-northd stopwatch/show northd)
+ echo "$stats" | grep -q 'Total samples: [[1-9]]'
+])
+
+dnl The "lr_nat" stopwatch should still have no samples.
+AT_CHECK([as northd ovn-appctl -t ovn-northd stopwatch/show lr_nat], [0], [dnl
+Statistics for 'lr_nat'
+ Total samples: 0
+ Maximum: 0 msec
+ Minimum: 0 msec
+ 95th percentile: 0.000000 msec
+ Short term average: 0.000000 msec
+ Long term average: 0.000000 msec
+])
+
+dnl Enable stopwatches globally (no argument).
+check as northd ovn-appctl -t ovn-northd inc-engine/enable-stopwatch
+check as northd ovn-appctl -t ovn-northd stopwatch/reset
+check as northd ovn-appctl -t ovn-northd inc-engine/recompute
+check ovn-nbctl --wait=sb sync
+
+dnl Now "lr_nat" should also have collected samples.
+OVS_WAIT_UNTIL([
+ stats=$(as northd ovn-appctl -t ovn-northd stopwatch/show lr_nat)
+ echo "$stats" | grep -q 'Total samples: [[1-9]]'
+])
+
+dnl Disable stopwatches only for the "northd" node.
+check as northd ovn-appctl -t ovn-northd inc-engine/disable-stopwatch northd
+check as northd ovn-appctl -t ovn-northd stopwatch/reset
+check as northd ovn-appctl -t ovn-northd inc-engine/recompute
+check ovn-nbctl --wait=sb sync
+
+dnl Verify "northd" has no new samples but "lr_nat" still collects.
+AT_CHECK([as northd ovn-appctl -t ovn-northd stopwatch/show northd], [0], [dnl
+Statistics for 'northd'
+ Total samples: 0
+ Maximum: 0 msec
+ Minimum: 0 msec
+ 95th percentile: 0.000000 msec
+ Short term average: 0.000000 msec
+ Long term average: 0.000000 msec
+])
+OVS_WAIT_UNTIL([
+ stats=$(as northd ovn-appctl -t ovn-northd stopwatch/show lr_nat)
+ echo "$stats" | grep -q 'Total samples: [[1-9]]'
+])
+
+dnl Disable stopwatches globally and verify no samples are collected.
+check as northd ovn-appctl -t ovn-northd inc-engine/disable-stopwatch
+check as northd ovn-appctl -t ovn-northd stopwatch/reset
+check as northd ovn-appctl -t ovn-northd inc-engine/recompute
+check ovn-nbctl --wait=sb sync
+AT_CHECK([as northd ovn-appctl -t ovn-northd stopwatch/show northd], [0], [dnl
+Statistics for 'northd'
+ Total samples: 0
+ Maximum: 0 msec
+ Minimum: 0 msec
+ 95th percentile: 0.000000 msec
+ Short term average: 0.000000 msec
+ Long term average: 0.000000 msec
+])
+
+dnl Error case: non-existent node name.
+AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/enable-stopwatch
nonexistent_node], [2], [], [ignore])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
--
2.54.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
