Skip to content

Wire psrpc bus compression into the message bus constructor - #831

Merged
paulwe merged 1 commit into
mainfrom
psrpc-bus-compression
Sep 6, 2026
Merged

paulwe merged 1 commit into
mainfrom
psrpc-bus-compression

Conversation

@paulwe

@paulwe paulwe commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

psrpc v0.7.6 (psrpc#130) added opt-in gzip at the bus boundary, and protocol (#1771) surfaced it as rpc.PSRPCConfig.Compression with a BusOptions() conversion. livekit-server (#4844), egress and ingress have all landed the wiring.

sip was the straggler, which mattered in two ways: it had no psrpc: config block at all, so the setting was unreachable here; and sip on psrpc v0.7.5 cannot decode a compressed payload — psrpc's subscription read loop drops undecodable messages without an error. Bumping sip is therefore a precondition for enabling compression anywhere on the shared bus.

psrpc:
  compression:
    quality: 6
    threshold: 1024

Compression is off by default

quality defaults to 0. Enabling it is a two-stage operator action: roll psrpc v0.7.6+ out to LiveKit server, egress, ingress, SIP and any agent workers first, then raise quality at the publishers. The README says so at the setting, along with a note that the other PSRPCConfig keys (max_attempts, timeout, backoff, buffer_size) parse but are unused here — NewConfig uses non-strict yaml.Unmarshal, so they'd otherwise silently look effective.

max_decompressed_size defaults to 0, unlimited, so nothing starts being dropped by a cap nobody set.

Shape

Follows egress (cmd/server/main.go:117) and ingress (cmd/server/main.go:132) exactly: a PSRPC rpc.PSRPCConfig field seeded with rpc.DefaultPSRPCConfig in the NewConfig literal before yaml.Unmarshal, so a partial override keeps the seeded threshold.

Seeded in NewConfig only, not Init(). A zero-valued rpc.PSRPCConfig is behaviourally identical to the old no-options bus — newCompressor returns nil for Quality <= 0, getBusOpts re-seeds Threshold to DefaultCompressionThreshold when it's <= 0, and MaxDecompressedSize: 0 is unlimited on read — so the two test paths that bypass NewConfig need no rescue. Init() also runs after NewConfig, so it couldn't assign wholesale without clobbering YAML values.

Both test bus call sites are wired too, so grep NewRedisMessageBus\|NewLocalMessageBus finds no stragglers. Neither changes behaviour.

Dependency bump

protocolv1.51.1-0.20260905133529-a4f4b5c0c23f and psrpcv0.7.6, the same revisions egress and ingress are on.

The protocol range carries one behavioural rider sip actually uses: TEL-955: fix potential panic in sip, which guards an empty INVITE user part in EvaluateDispatchRule. A fix sip wants. The rest are inert here — the grpc v1.83.1 security bump is already satisfied, sdp: harden SDPFragment.Unmarshal touches a package sip imports in zero files, and the billing-reporter and agent-simulation commits are additive protobuf in packages sip doesn't import.

go mod tidy also drops a stale golang.org/x/mod indirect requirement. That drift predates this change — running tidy on the unbumped deps produces the identical removal — but it rides along rather than being hand-restored.

Testing

  • go build ./..., go vet ./... and go test -race ./pkg/... pass; both test/ packages compile.
  • Verified the YAML reaches the bus: a partial override yields quality=6 threshold=1024 max=4096. threshold surviving at 1024 rather than resetting to 0 is what proves the seed-before-unmarshal ordering. Defaults yield quality=0 threshold=1024 max=0.
  • No pkg/config/config_test.go — this package has never had tests and this change doesn't introduce the first one. Worth knowing that leaves the seed-before-unmarshal ordering unguarded: moving the PSRPC: line below yaml.Unmarshal would silently reset threshold to 0 and start compressing every small message. Happy to add two short tests if reviewers want it closed.

Note on formatting

pkg/config/config.go and cmd/livekit-sip/main.go are already non-gofmt-clean on main, in regions untouched here (the MediaTimeout/RTPDraining alignment block, and the long one-liner at main.go:102). Running the formatter would have swept that unrelated churn into this diff, so I didn't — every line added here is independently goimports-clean. CI runs only staticcheck, no formatter, which is why the drift survives. Happy to fix it separately.

@paulwe
paulwe requested a review from a team as a code owner September 6, 2026 16:37

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@paulwe
paulwe requested a review from a team September 6, 2026 16:41
psrpc v0.7.6 added opt-in gzip at the bus boundary and protocol surfaced it
as rpc.PSRPCConfig.Compression with a BusOptions() conversion. livekit-server,
egress and ingress have all landed the wiring; sip had no psrpc: block at all,
so the setting was unreachable here.

More importantly, sip on psrpc v0.7.5 cannot decode a compressed payload — the
subscription read loop drops the message without an error — so bumping sip is a
precondition for enabling compression anywhere on the shared bus.

Compression stays off by default. Enabling it is a two-stage operator action:
roll psrpc v0.7.6+ out to every node on the bus first, then raise quality at the
publishers. The README says so at the setting.

go mod tidy also drops a stale golang.org/x/mod indirect requirement; that drift
predates this change and is not caused by the bump.
@paulwe
paulwe force-pushed the psrpc-bus-compression branch from 35730c8 to 3f84c6c Compare September 6, 2026 16:42
@paulwe
paulwe merged commit f7d4dab into main Sep 6, 2026
7 checks passed
@paulwe
paulwe deleted the psrpc-bus-compression branch September 6, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants