Skip to content

[MeshSync] Add opt-in content deduplication on the broker output path - #575

Merged
marblom007 merged 4 commits into
masterfrom
fix/meshsync-broker-dedup
Jul 5, 2026
Merged

marblom007 merged 4 commits into
masterfrom
fix/meshsync-broker-dedup

Conversation

@leecalcote

Copy link
Copy Markdown
Member

Description

Add an opt-in, content-hash deduplicator on the broker output path (internal/output/content_deduplicator.go, wired in pkg/lib/meshsync/meshsync.go).

  • Wraps BrokerWriter, keyed by resource UID, storing a sha256 of the last published payload. ADD/UPDATE skips a byte-identical republish; DELETE always publishes and evicts the UID (bounding the map); empty-UID objects always publish. The full object is always sent when published - the wire format is never changed to a delta, so meshery-server's full-object consumption is preserved.
  • Opt-in via MESHSYNC_BROKER_CONTENT_DEDUP (default OFF). The broker writer persists across informer resyncs, so a persistent dedup map could suppress the post-resync re-list that a recovery resync intends; making it opt-in guarantees default behavior is unchanged and no meaningful event is ever dropped.

Notes for Reviewers

go build ./..., go test ./..., and go test -race ./internal/output/... (incl. a 16-goroutine concurrency test) pass. Follow-up noted in the wrapper's doc comment: a resync-triggered cache reset would let dedup be enabled and fully resync-safe.

Signed commits

  • Yes, I signed my commits.

BrokerWriter.Write republished the full object to NATS on every
ADD/UPDATE/DELETE. resourceVersion-based suppression in the informer
UpdateFunc already drops no-op updates, but two distinct
resourceVersions can still carry byte-identical wire content, so those
redundant republishes waste broker bandwidth and Server DB writes.

Add ContentDeduplicatorWriter, a streaming pass-through wrapper that
keys resources by KubernetesResourceMeta.UID and remembers a sha256 of
the last published payload per UID:

- ADD/UPDATE: skip when the new payload hashes identical to the stored
  hash for that UID; otherwise publish and update the stored hash.
- DELETE: always publish and evict the UID, keeping the map bounded to
  live UIDs and letting a re-created UID republish fresh.
- Empty/absent UID: always publish, never deduplicate.
- Thread-safe via a mutex.

The full object is always sent when published; the wire format is never
rewritten into a delta/patch, since Meshery Server consumes full objects
from these subjects.

The wrapper is OFF by default and enabled via the
MESHSYNC_BROKER_CONTENT_DEDUP env var. The broker writer persists across
informer resyncs, and a resync is also a recovery path, so the safe
default is to republish everything; operators opt in to trade a small,
bounded amount of memory for reduced broker/DB churn.

Memory is bounded to one sha256 (32 bytes) per live resource UID:
entries are added on ADD/UPDATE and removed on DELETE.

Add unit tests covering identical-skip, republish-on-change,
always-emit-DELETE-and-evict, never-dedup-empty-UID, per-UID
independence, map bounding, and concurrent writes under -race.

Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
@github-actions github-actions Bot added the language/go Golang related label Jul 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces an opt-in content-hash deduplication mechanism (ContentDeduplicatorWriter) for the broker output path in MeshSync, controlled by the MESHSYNC_BROKER_CONTENT_DEDUP environment variable. This helps suppress byte-identical republishes of the same resource to reduce broker and database churn. The review feedback points out a performance bottleneck where a global mutex is held during CPU-heavy JSON serialization and network I/O, and suggests releasing the lock during these operations to prevent blocking concurrent writes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread internal/output/content_deduplicator.go Outdated
Hold w.mu only to guard the hash map; compute the sha256/JSON serialization
and call the downstream writer with the lock released, so concurrent writers
are not serialized by CPU-bound hashing or the broker publish. Addresses
review feedback on #575.

Signed-off-by: Lee Calcote <lee.calcote@layer5.io>

Copilot AI left a comment

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.

Pull request overview

Adds an opt-in, content-hash–based deduplication layer on the broker output path to suppress byte-identical re-publishes of the same resource (keyed by UID), reducing broker and downstream churn while keeping default behavior unchanged.

Changes:

  • Introduces ContentDeduplicatorWriter to hash JSON payloads per resource UID and skip identical ADD/UPDATE publishes (while always forwarding DELETE and evicting cache entries).
  • Wires the deduplicator into the broker output path behind MESHSYNC_BROKER_CONTENT_DEDUP (default OFF).
  • Adds unit tests, including a concurrency-focused test for distinct UIDs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pkg/lib/meshsync/meshsync.go Adds env-controlled opt-in wiring to wrap the broker writer with the new content deduplicator.
internal/output/content_deduplicator.go Implements the broker-output writer wrapper that deduplicates by hashing the JSON payload per UID.
internal/output/content_deduplicator_test.go Adds tests validating dedup behavior, delete eviction, UID handling, and concurrency.
internal/config/types.go Defines MESHSYNC_BROKER_CONTENT_DEDUP env var constant and documents its semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/output/content_deduplicator.go Outdated
Comment thread internal/output/content_deduplicator_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
Previously the content-hash was recorded before calling the downstream writer,
so a failed publish left the UID marked as published and suppressed a later
retry (or resync) of the same payload that had never actually shipped. Now the
payload is published first and the hash recorded only on success; a failed write
returns the error and leaves the hash unrecorded. Adds a regression test and
corrects a stale test comment. Addresses review feedback on #575.

Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
@marblom007
marblom007 merged commit b908c1b into master Jul 5, 2026
5 checks passed
@marblom007
marblom007 deleted the fix/meshsync-broker-dedup branch July 5, 2026 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

language/go Golang related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants