[fpmsyncd]: coalesce ZMQ route sends behind a dedicated send thread by deepak-singhal0408 · Pull Request #4855 · sonic-net/sonic-swss

deepak-singhal0408 · 2026-09-01T06:48:43Z

What I did

Decoupled FPM ingest from the ZMQ route send in fpmsyncd.

Route and label-route writes go into an in-memory coalescing map (last-writer-wins per key) that a dedicated send thread drains in chunks. A failed chunk is re-merged and retried, and never overwrites a newer update for the same key. A stall past the liveness bound writes a STATE_DB record and exits.

Scope:

Active only when SYSTEM_DEFAULTS|swss_zmq is enabled. Disabled, there is no map and no send thread.
Warm restart retires the coalescer. fpmsyncd drains and joins the send thread when the window opens, leaving reconcile the only writer of the route tables. checkAndStart() runs on every FPM reconnect, so warm restart armed mid-run is covered too.
VNET writes use a plain ProducerStateTable over Redis and bypass the coalescer.
EVPN shares the ZMQ-backed ROUTE_TABLE with unicast, so it coalesces alongside it.

Fixes sonic-net/sonic-buildimage#28369

Why I did it

ZmqProducerStateTable::set() is fire-and-forget, so a burst that hits the socket high-water mark drops the update silently after zebra has already been acknowledged. Holding the update until a send succeeds closes that window; coalescing falls out of it. Full problem statement in sonic-net/sonic-buildimage#28369.

How I verified it

24 new unit tests in routesendcoalescer_ut.cpp; tests_fpmsyncd 212/212, -Werror.
ZmqOrchTest, ZmqRouteOrchTest, ZmqRouteConsumerTest 27/27.
Each assertion checked by reverting the mechanism it covers and confirming the test fails.

Details if related

Design: sonic-net/SONiC#2481.

Depends on ZmqProducerStateTable's bounded send retry (sonic-swss-common#1233, merged) as the inner blip absorber; this PR adds the outer retry.
stop() drains the map to empty before joining, so no coalesced write lands after reconcile has decided the delta.
The coalescer drops no route. Chunks are bounded by entry count, matching every other producer in sonic-swss; swss-common enforces its own 16 MiB wire limit and throws, which the coalescer catches and retries.
A liveness failure is deliberately fatal: fpmsyncd exits, the bgp container bounces, and BGP re-convergence rebuilds the table. Recovery rests on topology redundancy, so this path targets T1 and above.

Telemetry is in STATE_DB under the route-stat record: episode counters, stall gauges, last_success_age_sec, routes_lost_total.

azure-pipelines · 2026-09-01T06:48:52Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

mssonicbld · 2026-09-01T06:49:02Z

/azp run

azure-pipelines · 2026-09-01T06:49:14Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

This PR updates fpmsyncd to avoid silent route drops on the ZMQ route path by buffering steady-state route/label-route updates in an in-memory last-writer-wins coalescing map that a dedicated send thread drains in bounded chunks, with retry/backoff, liveness asserts, and STATE_DB telemetry.

Changes:

Add RouteSendCoalescer (map + send thread) to coalesce and reliably retry ZMQ route sends, with congestion/health telemetry and deliberate-exit on prolonged stalls.
Integrate the coalescer into RouteSync for the ZMQ route path, including warm-restart pause/resume to preserve single-writer semantics.
Add extensive mock unit tests for coalescing, chunking, retry/episode behavior, liveness asserts, and telemetry schema.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/mock_tests/Makefile.am	Adds the new coalescer unit test and implementation file to the fpmsyncd mock test build.
tests/mock_tests/fpmsyncd/routesendcoalescer_ut.cpp	New unit tests covering coalescing, chunking, retries, liveness, pause/resume, and telemetry.
fpmsyncd/routesync.h	Wires in coalescer/state DB members and helper methods for the ZMQ-backed tables.
fpmsyncd/routesync.cpp	Routes steady-state ZMQ table writes through the coalescer; pauses during warm-restart reconcile; stops thread on teardown.
fpmsyncd/routesendcoalescer.h	New coalescer class interface (config, ingest APIs, thread lifecycle, telemetry accessors).
fpmsyncd/routesendcoalescer.cpp	New coalescer implementation (bounded drains, retry/re-merge, episode tracking, telemetry, liveness assert/exit).
fpmsyncd/Makefile.am	Adds `routesendcoalescer.cpp` to the fpmsyncd build.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mssonicbld · 2026-09-01T06:56:55Z

/azp run

azure-pipelines · 2026-09-01T06:57:07Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-01T06:58:13Z

/azp run

azure-pipelines · 2026-09-01T06:58:24Z

Azure Pipelines: Successfully started running 1 pipeline(s).

deepak-singhal0408 · 2026-09-01T16:29:22Z

Hi @venkit-nexthop @prabhataravind, Could you help review this PR.

anarasimhan-upscale

few comments

prabhataravind · 2026-09-15T14:35:15Z

@deepak-singhal0408 The PR description says "VNET and EVPN route handlers are untouched," but that's only true for VNET (onVnetRouteMsg writes via setTable(), which has no coalescerActive() check). onEvpnRouteMsg itself has no diff lines, but it calls setRouteWithWarmRestart(fvw, *m_routeTable), which this PR did modify — so once swss_zmq is enabled, EVPN route writes to ROUTE_TABLE flow through the coalescing map and send thread just like plain unicast routes. Can you confirm whether EVPN is intentionally in scope (and the description just needs a wording fix) or whether it was meant to be excluded like VNET?

mssonicbld · 2026-09-16T00:26:02Z

/azp run

azure-pipelines · 2026-09-16T00:26:14Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-16T04:20:36Z

/azp run

azure-pipelines · 2026-09-16T04:20:48Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-16T04:57:35Z

/azp run

azure-pipelines · 2026-09-16T04:57:46Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-16T05:00:29Z

/azp run

azure-pipelines · 2026-09-16T05:00:51Z

Azure Pipelines: Successfully started running 1 pipeline(s).

deepak-singhal0408 · 2026-09-16T05:19:56Z

@deepak-singhal0408 The PR description says "VNET and EVPN route handlers are untouched," but that's only true for VNET (onVnetRouteMsg writes via setTable(), which has no coalescerActive() check). onEvpnRouteMsg itself has no diff lines, but it calls setRouteWithWarmRestart(fvw, *m_routeTable), which this PR did modify — so once swss_zmq is enabled, EVPN route writes to ROUTE_TABLE flow through the coalescing map and send thread just like plain unicast routes. Can you confirm whether EVPN is intentionally in scope (and the description just needs a wording fix) or whether it was meant to be excluded like VNET?

@prabhataravind good catch. The PR description was wrong. Fixed it. EVPN was already on the ZMQ path before this PR (when zmq is enabled), so same behavior remains, whereas VNET stays on REDIS path as before.

mssonicbld · 2026-09-18T21:04:01Z

/azp run

azure-pipelines · 2026-09-18T21:04:12Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Under a routing burst, FRR/FPM ingest can outrun the ZMQ route path and overflow the send socket, silently dropping routes. Decouple ingest from the ZMQ send: steady-state route and label-route writes are coalesced (last-writer-wins per key) into an in-memory map and drained in chunks by a dedicated send thread, so ingest never blocks on a full ZMQ socket and a failed chunk is retried from the map rather than lost. - RouteSendCoalescer: coalescing map plus send thread, chunked drain under entry and byte caps, retry with backoff, and STATE_DB health telemetry (episode counters, congestion/stall gauges, RECOVERED edge). Each table is drained every cycle so neither starves the other, and liveness is tracked per table so a stall in one is visible while the other flows. - routesync funnels the steady-state ZMQ-table write path through the coalescer. The non-ZMQ path and the offload-reply path are unchanged. - Warm-restart reconciliation writes the route tables directly, so the send thread is parked for its duration, keeping a single writer at any instant. Warm restart re-enters on FPM reconnect, so this is an explicit pause/resume rather than a startup-only assumption. - An entry too large to ever fit in a ZMQ message is dropped and counted instead of being retried until the liveness guard fires. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0e1be286-79be-4087-a72b-64836072e65e Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>

Rework the coalescer in response to review. The net effect is a smaller surface: the pause/resume handshake, three asserts, a drop counter and two size caps are gone, and none of them are replaced by new machinery. Make the ZMQ route path mutually exclusive with warm and fast restart, and enforce it where the path is configured rather than where it is read: - get_route_perf_zmq_enabled() is now a plain read of SYSTEM_DEFAULTS and never throws. validate_route_perf_zmq_supported() holds the check and is called from create_route_perf_zmq_client() and orchdaemon, so an unsupported combination fails at startup instead of mid-run. - With that guarantee, warm-restart reconciliation can no longer overlap the send thread, so the pause/resume handshake is deleted. The send thread is the single writer by construction. This also removes a latent out-of-bounds read: the warm arm of setRouteWithWarmRestart indexes element [1] of the wrapper vector, which only exists in the non-ZMQ layout. Replace the three asserts with a drop plus SWSS_LOG_ERROR. fpmsyncd builds with DBGFLAGS = -g while the mock tests build with -g -DNDEBUG, so the asserts were live exactly where they were never exercised and compiled out where they were. Remove both size caps, which could not fire for a real route entry: - The oversize-entry drop and maxWireBytes discarded entries the wire would have accepted. ZmqClient::sendMsg already throws on a genuinely oversized message, which the coalescer catches, re-merges and retries. - The per-chunk byte cap and its size helper are unreachable behind the 256-entry cap, which bounds a chunk to ~7.5 MiB against a 16 MiB limit. The helper also walked every field and allocated per entry while holding the ingest mutex. No other producer in sonic-swss does byte accounting before a send; bounding by entry count matches the convention. Drop malformed_dropped_total. Malformed input is a bug to fix, not a rate to watch, and the error log already names the offending key. Also address review nits: pass the KCO vector by const reference and add the includes the unit test and header relied on transitively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>

SWSS_LOG_THROW was uncaught in every caller, so an unsupported configuration terminated orchagent and fpmsyncd with a core dump instead of refusing to start. Replace the throwing validator with a predicate and check it at the three entry points that configure the path: OrchDaemon::init(), fpmsyncd main() and routeresync main(). Each logs and exits non-zero. In orchagent the check moves to the top of init(). It previously sat after FgNhgOrch, whose constructor configures the path, so the throw from that constructor fired first and the explicit check was unreachable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>

mssonicbld · 2026-09-22T20:08:31Z

/azp run

…ing it The premise behind the startup guard was wrong. ZMQ and warm restart have always worked together: ZmqProducerStateTable is constructed with dbPersistence enabled, so every ZMQ set() mirrors to APPL_DB and the restoration read finds its content, and WarmStartHelper is handed the ZMQ table itself, so reconcile writes back over ZMQ. Refusing the combination regressed a supported configuration. Drop the guard and the lib/orch_zmq_config.h helpers it added, restoring orchagent, routeresync and fpmsyncd to their prior startup paths. The real constraint is narrower: reconcile writes the route tables on the main thread, so the send thread must not be writing them at the same time. fpmsyncd now retires the coalescer when warm restart opens its window and starts it otherwise. stop() already drains the map to empty before joining, so no coalesced write can land after reconcile has decided the delta. Since checkAndStart() runs on every FPM reconnect, this also covers warm restart armed after the process is already coalescing. stop() now reports whether it joined a running thread, which keeps the retire log honest when the coalescer was constructed but never started. Restore the warm-restart branch of setRouteWithWarmRestart to its upstream form. The ZMQ wrapper emits a single SET, so the tuple lives at [0] and the hardcoded [1] this PR had introduced was out of bounds on that path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>

azure-pipelines · 2026-09-22T20:08:43Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-22T20:10:55Z

/azp run

azure-pipelines · 2026-09-22T20:11:07Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot AI lite review requested due to automatic review settings September 1, 2026 06:48

deepak-singhal0408 requested a review from prsunny as a code owner September 1, 2026 06:48

Copilot started reviewing on behalf of deepak-singhal0408 September 1, 2026 06:49 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread fpmsyncd/routesendcoalescer.h

Comment thread tests/mock_tests/fpmsyncd/routesendcoalescer_ut.cpp

Comment thread fpmsyncd/routesync.cpp Outdated

deepak-singhal0408 self-assigned this Sep 1, 2026

deepak-singhal0408 force-pushed the fix-28369-prB-coalescing-sendthread branch from a43dadf to 2336b7e Compare September 1, 2026 06:56

deepak-singhal0408 force-pushed the fix-28369-prB-coalescing-sendthread branch from 2336b7e to 4157f10 Compare September 1, 2026 06:57

deepak-singhal0408 requested a review from prabhataravind September 1, 2026 16:28

deepak-singhal0408 added this to SONiC Routing Dashboard Sep 3, 2026

github-project-automation Bot moved this to Todo in SONiC Routing Dashboard Sep 3, 2026

deepak-singhal0408 added this to SONiC Chassis Sep 3, 2026

deepak-singhal0408 moved this to In Progress in SONiC Chassis Sep 3, 2026

deepak-singhal0408 moved this from Todo to In Progress in SONiC Routing Dashboard Sep 3, 2026

venkit-nexthop reviewed Sep 4, 2026

View reviewed changes

Comment thread fpmsyncd/routesync.cpp Outdated

anarasimhan-upscale reviewed Sep 5, 2026

View reviewed changes

Comment thread fpmsyncd/routesendcoalescer.cpp Outdated

Comment thread fpmsyncd/routesendcoalescer.cpp Outdated

Comment thread fpmsyncd/routesync.cpp Outdated

Comment thread fpmsyncd/routesendcoalescer.cpp Outdated

prabhataravind reviewed Sep 15, 2026

View reviewed changes

Comment thread fpmsyncd/routesendcoalescer.cpp

prabhataravind reviewed Sep 15, 2026

View reviewed changes

Comment thread fpmsyncd/routesync.cpp

prabhataravind reviewed Sep 15, 2026

View reviewed changes

Comment thread fpmsyncd/routesendcoalescer.cpp Outdated

deepak-singhal0408 force-pushed the fix-28369-prB-coalescing-sendthread branch from 986988b to 6a305cd Compare September 16, 2026 04:57

deepak-singhal0408 force-pushed the fix-28369-prB-coalescing-sendthread branch from 6a305cd to 412cbee Compare September 16, 2026 05:00

deepak-singhal0408 mentioned this pull request Sep 17, 2026

Refuse warm/fast restart while the ZMQ route path is enabled sonic-net/sonic-utilities#4857

Closed

prabhataravind reviewed Sep 18, 2026

View reviewed changes

Comment thread lib/orch_zmq_config.cpp Outdated

anarasimhan-upscale reviewed Sep 18, 2026

View reviewed changes

Comment thread fpmsyncd/fpmsyncd.cpp Outdated

deepak-singhal0408 and others added 3 commits September 22, 2026 19:50

deepak-singhal0408 force-pushed the fix-28369-prB-coalescing-sendthread branch from 9b230d0 to 5fc7e22 Compare September 22, 2026 20:08

ashutosh-agrawal mentioned this pull request Sep 23, 2026

[DVS] test_mux_prefixroute repeatedly exhausts retries after #4886 #4922

Open

Conversation

deepak-singhal0408 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

mssonicbld commented Sep 1, 2026

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Sep 1, 2026

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

mssonicbld commented Sep 1, 2026

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

deepak-singhal0408 commented Sep 1, 2026

Uh oh!

Uh oh!

anarasimhan-upscale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prabhataravind commented Sep 15, 2026

Uh oh!

mssonicbld commented Sep 16, 2026

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

mssonicbld commented Sep 16, 2026

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

mssonicbld commented Sep 16, 2026

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

mssonicbld commented Sep 16, 2026

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

deepak-singhal0408 commented Sep 16, 2026

Uh oh!

Uh oh!

mssonicbld commented Sep 18, 2026

Uh oh!

azure-pipelines Bot commented Sep 18, 2026

Uh oh!

Uh oh!

mssonicbld commented Sep 22, 2026

Uh oh!

azure-pipelines Bot commented Sep 22, 2026

Uh oh!

mssonicbld commented Sep 22, 2026

Uh oh!

azure-pipelines Bot commented Sep 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

deepak-singhal0408 commented Sep 1, 2026 •

edited

Loading