Repository navigation
Conversation
The server classifies any connection with a live subscription as a pub/sub client and applies the pubsub output-buffer limits to it (32mb hard / 8mb for 60s by default). Since #3255 subscribed the configuration channel on the shared RESP3 interactive connection, ordinary large replies or pipelined bursts could get that connection - and every in-flight command on it - closed by the server. Add ConfigurationOptions.SharedSubscriptionConnection (sharedSubscriptionConnection=, default false): under RESP3, pub/sub now gets its own connection as under RESP2, unless sharing is opted into. - route via ServerEndPoint.SharesSubscriptionConnection() rather than KnowOrAssumeResp3(); connect monitors wait for the second leg - send QUIT once when the connection is shared - tests cover both modes; new PubSubOutputBufferTests reproduces the server-side kill with an oversized MGET, without changing server config - docs: Resp3.md / Configuration.md explain the limits and advise checking the server's pubsub limits before opting in
CI saw a third socket (two Subscription sockets at the same timestamp): ActivateServer and OnFullyEstablished both lazily created the subscription bridge via a non-atomic `??=`, and a bridge connects as soon as it is created, so the loser leaked an unreferenced connection. The previous commit made OnFullyEstablished call Activate(Subscription) on every non-shared RESP3 connect, widening a previously rare race. - GetOrCreateBridge: CompareExchange the new bridge in; the loser is disposed (closing its connection) - OnFullyEstablished only activates pub/sub for the shared-RESP3-but-got- RESP2 case it was written for; otherwise ActivateServer has done it
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3263.
Problem
Redis classifies any connection with a live subscription as a pub/sub client (
flags=P), and applies thepubsubclass ofclient-output-buffer-limitto it -32mb 8mb 60by default, against no limit for normal clients. #3255 (unreleased) correctly subscribed the configuration channel under RESP3, but because RESP3 shared the interactive connection with pub/sub, that made every RESP3 interactive connection a pub/sub client. A single large reply, a largeMGET/HGETALL, or a burst of pipelined replies then gets the connection closed by the server, failing every in-flight command. This was never specific to the config channel: any RESP3 user who subscribed to anything was exposed; #3255 just made it the default.Measured against Redis 8.9 with default limits:
NNSUBSCRIBEPSUBSCRIBEthenUNSUBSCRIBENThe whole reply is checked against the hard limit before it is written, so
MGETof a 64KiB value ~600 times reproduces it just as well.Change
ConfigurationOptions.SharedSubscriptionConnection/sharedSubscriptionConnection=(also onDefaultOptionsProvider), default false. Under RESP3, pub/sub now uses a dedicated connection (negotiating RESP3 itself), exactly as under RESP2; sharing the interactive connection is opt-in. No effect under RESP2. No provider overrides the default.ServerEndPoint.SharesSubscriptionConnection()instead ofKnowOrAssumeResp3(); with sharing off, routing no longer depends on the pre-handshake protocol guess at all. Connect monitors wait for the second leg unless shared.QUITto the shared connection on close (Close(Subscription)resolved to the interactive bridge).Resp3.md(new "Pub/sub connections" section),Configuration.md,SyncOverAsync.md- including advice to check the server'spubsublimits against the largest replies/pipelined bursts before opting in.Tests
PubSubOutputBufferTests: reads the server's pubsub hard limit (skips if unavailable), subscribes, thenMGETs past it; dedicated connection survives and staysNormal, shared RESP3 connection is killed. No server config change, so it can run in parallel.MaintenanceNotificationTests: four exact delivery counts relaxed to> 0, since the test server'sSendRawPush(null, ...)deliberately ignores opt-in and now also reaches the RESP3 subscription connection.Full suite green on net10.0 locally; on net8.0 one unrelated timing flake (
KeyIdleAsyncTests.TouchIdleTimeAsync, passes in isolation). net481 not run locally.Release notes (for the GitHub release)