Skip to content

redis: add pub/sub support via RESP3 Push messages#46354

Closed
bellatoris wants to merge 4 commits into
envoyproxy:mainfrom
bellatoris:doogie/resp3-pubsub
Closed

redis: add pub/sub support via RESP3 Push messages#46354
bellatoris wants to merge 4 commits into
envoyproxy:mainfrom
bellatoris:doogie/resp3-pubsub

Conversation

@bellatoris

@bellatoris bellatoris commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Commit Message:

redis: add pub/sub support via RESP3 Push messages

Additional Description:

Follow-up to #44936, which left SUBSCRIBE forwarding as future work. Adds pub/sub to redis_proxy on RESP3 listeners, built on Redis 7 sharded pub/sub: downstream SUBSCRIBE/UNSUBSCRIBE are rewritten upstream to SSUBSCRIBE/SUNSUBSCRIBE, so subscriptions route by cluster slot exactly like keys, and messages reach the client as RESP3 Push frames. A per-worker SubscriptionRegistry owns placement and healing: each channel has one owning upstream host on a dedicated subscription connection, subscribe attempts are generation-tagged and correlated to their out-of-band acks through a per-host FIFO ledger, and upstream connection loss, -MOVED/-ASK on control commands, failover, and slot migration all re-place and re-subscribe with backoff — invisible to the downstream client. Acks are delivered at their position in the request FIFO, pushes never interleave inside pipelined replies, and slow subscribers are bounded by watermark checks plus eviction (the proxy-side analogue of client-output-buffer-limit). Placement is PRIMARY (default) or SHARD_MEMBERS (least-loaded across the shard's primary and replicas); PUBLISH can opt into SPUBLISH rewriting via enable_sharded_publish.

name: envoy.filters.network.redis_proxy
typed_config:
  "@type": type.googleapis.com/envoy.extensions.filters.network.redis_proxy.v3.RedisProxy
  stat_prefix: redis
  protocol_version: RESP3   # pub/sub requires the RESP3 listener mode
  settings:
    op_timeout: 5s
    pubsub_settings: {}     # defaults: 10s ack timeout, 100ms–30s backoff, PRIMARY placement

Design note: sharded-only, deliberately. Classic broadcast pub/sub (SUBSCRIBE fan-out to every node, PSUBSCRIBE patterns) has no slot affinity, so a proxy in front of a cluster cannot place or heal it coherently. PSUBSCRIBE/PUBSUB/RESET remain rejected; the sharded verbs are not client-exposed (and the pub/sub names can no longer be declared in custom_commands); sharded_subscription_mode: DISABLED restores the pre-feature rejection for Redis 6.x upstreams. Separately, commands that fail to queue inside a MULTI (unknown, wrong arity, locally-answered, malformed frames, routing failures) now dirty the transaction so EXEC aborts with -EXECABORT, matching real Redis CLIENT_DIRTY_EXEC semantics — this applies to all listeners and is changelogged.

Risk Level: Medium. Pub/sub is opt-in (protocol_version: RESP3 plus actual subscribe traffic); the RESP2 default path is unchanged. The transaction dirty-EXEC corrections apply to all listeners and are listed in the changelog.

Testing: Unit + integration suites (registry state machine incl. generation/FIFO/topology handling, conn-pool subscription connections, transaction×pubsub boundary, downstream ordering and watermarks). Additionally verified against a real Redis 7.2 cluster (3 primaries + 3 replicas), including subscribe continuity across failover and live slot resharding.

Docs Changes: redis_proxy_filter.rst (pub/sub configuration + new stats), arch overview redis page, API proto docs.

Release Notes: changelogs/current/new_features (pub/sub support), minor_behavior_changes (transaction dirty-EXEC), behavior_changes (pub/sub names rejected in custom_commands).

Runtime guard: N/A — feature is config-gated.

Platform Specific Features: N/A

I used generative AI to help author this change.

Add SUBSCRIBE/PSUBSCRIBE/SSUBSCRIBE forwarding in the redis_proxy filter
and RESP3 Push-frame delivery back to subscribed downstream clients.
Builds on the RESP3 init pipeline introduced in lyyvkozu.

* port the SUBSCRIBE pipeline onto Client::sendCommand and the new
  PushMessageCallbacks plumbing
* unify ClientImpl's held queue across user requests and fire-and-forget
  commands so SUBSCRIBE issued during HELLO 3 / READONLY / IAM token fetch
  is held and replayed in original submission order
* sendCommand now respects both the init gate and queue_enabled_; replay
  uses absl::variant + absl::visit to dispatch each entry by type
* onRespValue routes Push frames to push_callbacks_ when set, drops them
  with a debug log otherwise, and tears the connection down on a non-Push
  reply with empty pending_requests_ instead of asserting (Rule 5)
* onEvent(Connected) no longer asserts on empty pending_requests_, so the
  AWS IAM pending-token case can connect cleanly
* subscriptionRegistryShared() gates on the typed enum upstream protocol
  version, getOrCreateSubscriptionClient is just lookup + setPushCallbacks,
  and conn_pool_impl uses Common::Redis::Utility::makeRequest in place of
  the previously-undefined buildCommand helper
* remove plumbing made redundant by the init-pipeline refactor:
  sendAwsIamAuth public API, queueRequests, makeRequestImmediate,
  Hello3Callbacks, hello3_sent_hosts_, the uint32 protocol_version_ field,
  and the downstreamRespVersion / upstreamProtocolVersion /
  setExplicitHelloVersion / explicitHelloVersion accessors
* add 6 focused tests covering sendCommand held during AwaitingHello /
  AwaitingReadonly / WaitingForAwsToken, mixed user+sendCommand FIFO replay,
  Push delivery with empty pending, and non-Push reply close

Signed-off-by: Doogie Min <doogie.min@sendbird.com>
…Y fixed, behavior-invariant) — recordedOwnerValid/resolvePlacement helpers, reissueSsubscribe + onClusterTopologyChange rewritten to 'record valid → retarget / invalid → re-place', synthetic sendUpstreamSsubscribe removed (fresh subscribe → resolvePlacement+sendToHost+record). All existing suites green.

Signed-off-by: Doogie Min <doogie.min@sendbird.com>
Round-5 must-fix/recommended (prior): H1/H2 (C-1 DNS-Loading regression: pop-cascade hang + cancel UAF, one in_dns_redirect_ state flag), H3 doc (health-escape depends on Host health flipping), M1 (non-cluster validity degrades to PRIMARY), M2 (backoff coalesce vs stale pool timer), M3 (CLUSTER SLOTS range hardening), M4 (ValidityMode enum), M5 (whole-shard-unhealthy anti-ping-pong), A2 (drop speculative as_primary/ShardMembers::primary), A3/A4 helpers, §3.1/§3.2.

Deferred batch (this pass, all Docker-green):
- §5 efficiency: cache ShardMembershipResolver (dyn_cast once per cluster update), zero-copy membersForSlot, reuse validity candidates for SHARD_MEMBERS re-placement (E-8 parity), allocation-free reservoir tie-break in leastLoadedOf, addSubscription returns {subscriber_newly_added, first_subscriber} (one hash), absl::c_linear_search for membership scans, resetResubscribeCycle() helper, relocate RedirectionValues to client.h and reuse MOVED/ASK/CLUSTERDOWN in the registry.
- §3.3: trim stale "former X was removed" comment narration (5 sites).
- A1: remove prod-dead onUpstreamConnectionClose (onEvent open-codes the 3 primitives for weak-capture posting); drop the null-host whole-registry branch + ASSERT non-null; port tests to a simulateUpstreamConnectionClose helper.
- M6: scope a -MOVED re-mark to the correlated channel (not the whole host) so a slot-migration pre-refresh window does not churn a host every backoff; whole-host fallback only for an uncorrelated (empty-FIFO) redirect; backoff+refresh remain the safety net.

Regression tests added; unit targets Docker-green.

Review follow-up: onClusterRemoval now drains pending_requests_ so a request parked mid DNS-redirect (in_dns_redirect_, attached to no client) is failed via ~PendingRequest instead of awaiting a DNS continuation for a gone cluster (finding 1, regression test). Redirect classifier matches the leading error-code TOKEN (code + space/end), not a bare prefix, so "MOVEDX ..." is not misread as -MOVED (finding 3, regression test).

Review follow-up 2: shardCandidatesForChannel health-tiers Healthy -> Degraded -> all (was Healthy -> all), matching Envoy LB semantics — a degraded-but-usable member is preferred over an unhealthy one, and the tiering also lets a failure-driven escape leave an unhealthy owner without waiting for the whole shard to go down (test extended with mixed/degraded/all-fallback cases). Also stripped all internal review-tracking labels (D1/S6-x/E-8/§N/Issue N/Finding N/#N/...) from the pub/sub stack's comments so upstream readers see self-contained rationale, not review-round shorthand; comment-only, unit targets Docker-green.

Review follow-up 4: (1) documented the pub/sub delivery-ordering caveat in redis.rst (an out-of-band unsubscribe ack can precede a message parked behind a pipelined command reply for the same channel — within best-effort delivery, but strict-state-machine clients should not pipeline commands with an in-flight UNSUBSCRIBE); (2) subscription_registry_lib include/deps hygiene — added direct includes (envoy/network/connection.h, source/common/buffer/buffer_impl.h) and matching BUILD deps (network/stats/upstream/common:buffer/common:random_generator) for types the header uses directly; (3) inclusive language — 'master' -> 'primary' in explanatory comments (MASTER enum literals unchanged); (4) dropped internal review-history prose ('round 3', 'F-series') from two comments.

Signed-off-by: Doogie Min <doogie.min@sendbird.com>
@repokitteh-read-only

Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #46354 was opened by bellatoris.

see: more, trace.

@bellatoris
bellatoris force-pushed the doogie/resp3-pubsub branch 5 times, most recently from 711b090 to 66dd4f1 Compare July 24, 2026 02:31
@repokitteh-read-only

Copy link
Copy Markdown

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/coverage.yaml).
envoyproxy/coverage-shephards assignee is @RyanTheOptimist

🐱

Caused by: #46354 was synchronize by bellatoris.

see: more, trace.

@bellatoris
bellatoris force-pushed the doogie/resp3-pubsub branch 3 times, most recently from 1229e17 to 56ec800 Compare July 24, 2026 03:06
…ol acks + doc/marker fixes

Signed-off-by: Doogie Min <doogie.min@sendbird.com>
@bellatoris
bellatoris force-pushed the doogie/resp3-pubsub branch from 56ec800 to 5a4b7ab Compare July 24, 2026 13:46
@bellatoris bellatoris closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants