[show, clear]: add 'orchagent tasks' commands to surface per-Executor timing + scheduling latency#4536
Conversation
|
|
Surface per-Executor runtime and scheduling-latency statistics from orchagent without rebuilding. Operators can now answer "where is orchagent spending its time?" from the CLI. New commands: - show orchagent tasks -> per-Executor run-time + sched-latency table - sonic-clear orchagent tasks -> reset the counters in place The CLI talks to orchagent over two APPL_DB notification channels (ORCH_TASK_STATS_QUERY / ORCH_TASK_STATS_REPLY) and parses the 14-field pipe-separated reply. Rows are sorted by total run-time descending; the RUN TIME and SCHED LATENCY columns render P^2 median/q1/q3/max quartets in milliseconds, the OUTLIERS column shows Tukey 1.5x IQR sums, and the TOTAL column shows cumulative run/sched wall-clock. Tests mock swsscommon so the round-trip is exercised without Redis. Coverage: header presence, sort order, quartet formatting, outlier counts, empty/zero-slot rendering, timeout/error paths, and that the correct op string is sent. doc/Command-Reference.md is updated with the new Orchagent section. This is the CLI half; the orchagent-side instrumentation that emits the reply lives in a companion sonic-swss change. Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
b0342e7 to
7019adc
Compare
|
@prsunny @prabhataravind @vivekrnv @deepak-singhal0408 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collapse alignment padding in show/orchagent.py assignments and the show_orchagent_tasks_test.py assert statements that tripped flake8's E221 (multiple spaces before operator) and E272 (multiple spaces before keyword) checks in the Pretest Static Analysis stage. Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@deepak-singhal0408 @prsunny |
deepak-singhal0408
left a comment
There was a problem hiding this comment.
Please take care of multi-asic case as well.
There was a problem hiding this comment.
Pull request overview
This PR adds new SONiC CLI plumbing to query orchagent for per-Executor timing statistics and to reset those counters, enabling operators to see where orchagent spends time without rebuilding the daemon.
Changes:
- Add
show orchagent tasksto query orchagent via APPL_DB notifications, parse the reply, and render a timing/latency table. - Add
sonic-clear orchagent tasksto request clearing the same counters and report success/failure. - Document the new commands and add unit tests that mock the swsscommon notification round-trip.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/show_orchagent_tasks_test.py | Adds unit tests covering rendering, sorting, formatting, and error paths for show orchagent tasks. |
| tests/clear_orchagent_tasks_test.py | Adds unit tests for sonic-clear orchagent tasks request/timeout/error behavior. |
| show/orchagent.py | Implements the show orchagent tasks click group/command and notification query/parse/render pipeline. |
| show/main.py | Wires the new orchagent show group into the top-level show CLI. |
| clear/orchagent.py | Implements the sonic-clear orchagent tasks click group/command and notification clear request. |
| clear/main.py | Wires the new orchagent clear group into the top-level sonic-clear CLI. |
| doc/Command-Reference.md | Documents the new Orchagent section and both commands (usage + example output). |
- Make 'show/clear orchagent tasks' namespace-aware: add -n/--namespace, iterate multi_asic.get_namespace_list(), open APPL_DB per-namespace (4-arg DBConnector), and render an ASIC column when multi-ASIC. Fixes the 10s timeout on multi-ASIC (orchagent runs per-ASIC in asicN namespaces); single-ASIC behavior is unchanged and a fabric supervisor becomes a graceful no-op. - _parse_stats: tolerate a trailing '|' and warn to stderr on skipped rows instead of silently producing an incomplete table. - Fix clear test module docstring; add multi-ASIC + malformed-row unit tests. Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Bring the branch up to date with upstream master. Resolve the orchagent command registration in clear/main.py to keep two blank lines after the newly-added llr_counters function definition (flake8 E305). Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
can you take a look now. |
…with -n Enhance the Command-Reference orchagent section to cover multi-ASIC: per-ASIC namespaces, the aggregated ASIC column, -n/--namespace usage, and the supervisor no-op, with single- and multi-ASIC examples for both show and clear. Also make 'show orchagent tasks' render the ASIC column whenever the platform is multi-ASIC (via is_multi_asic()) rather than only when more than one namespace is queried, so selecting a single ASIC with -n keeps the owning ASIC explicit and stays consistent with the clear command's per-namespace labeling. Add a unit test for that case. Signed-off-by: Venkit Kasiviswanathan <venkit@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
What I did
Add two new CLI commands that surface orchagent's per-Executor execution-time
statistics so operators can answer "where is orchagent spending its time?"
without rebuilding it:
show orchagent tasks— per-Executor run-time and scheduling-latency table.sonic-clear orchagent tasks— reset the counters in place.This is the CLI half; the orchagent-side instrumentation that publishes the
reply lives in a companion
sonic-swsschange. The CLI degrades gracefully(times out with a non-zero exit code) if the daemon side is not present.
How I did it
Two new click groups (
show.orchagent.tasksandclear.orchagent.tasks)wired into
show/main.pyandclear/main.py.The CLI talks to orchagent over two APPL_DB notification channels
(
ORCH_TASK_STATS_QUERY/ORCH_TASK_STATS_REPLY) and parses the14-field pipe-separated reply produced by orchagent:
Rendering:
median/q1/q3/maxof per-invocation wall-clock duration.Median is the headline, Q1/Q3 give spread, max exposes the worst tail.
median/q1/q3/maxof the gap between when the taskfinished and when it was next scheduled. Exposes select-loop starvation.
<run>/<sched>wall-clock spent inside the task vswaiting before it.
Rows are sorted by
total_run_nsdescending; ties break by name ascending.Empty slots (count = 0) print
-for quartet/total cells but keep integerRUNS/OUTLIERS.sonic-clear orchagent taskssendsop="clear"on the same query channeland prints
OKon a successful round-trip.doc/Command-Reference.mdis updated with a new Orchagent section(TOC + body) per the contributing guideline for new
show/sonic-clearsubcommands.
How to verify it
tests/show_orchagent_tasks_test.pyandtests/clear_orchagent_tasks_test.pymock
swsscommon.NotificationProducer/NotificationConsumer/Selectso the round-trip is exercised end-to-end without Redis. Coverage:
SCHED LATENCY / TOTAL / run/sched / (in msec))
total_run_nsdescending, name asc tiebreak)<run>/<sched>with empty-row dashesshow/clear)12 / 12 pytests pass against the mocked swsscommon round-trip.
Manual: with the companion
sonic-swsschange loaded, run the commands ona live switch and observe per-Executor stats / reset.
New command output (if the output of a command-line utility has changed)