SY-4432: Refactor Distribution Proxy And Transport#2531
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc #2531 +/- ##
==========================================
+ Coverage 68.77% 68.84% +0.06%
==========================================
Files 2608 2622 +14
Lines 129273 129471 +198
Branches 9310 9302 -8
==========================================
+ Hits 88912 89131 +219
+ Misses 33649 33630 -19
+ Partials 6712 6710 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…o services Bring over the new proxy and distribution transport test suites, document the public transport types/methods, and fix bugs surfaced by the tests: the gRPC framer deleter client had a nil Exec, the framer deleter server was never bound, server-side (and deleter) middleware was never wired into Use, and channel Use skipped rename. Mark internal node-to-node servers as Internal for richer error encoding. Rename the distribution channel gRPC services ChannelCreateService/ ChannelDeleteService/ChannelRenameService to CreateService/DeleteService/ RenameService and regenerate the protobuf stubs.
Add doc comments to the distribution channel Transport interface, its operation transports, and the create/delete/rename request payloads (and their fields), and to the channel proto services/messages in services.proto (regenerated). Document the remaining exported methods across the distribution transport gRPC and mock implementations so every public identifier carries a comment.
Drop the Transport infix and Unary prefix from the per-operation transport type aliases: channel exposes CreateClient/CreateServer, DeleteClient/DeleteServer, and RenameClient/RenameServer; the unary deleter exposes bare Client/Server. Group each channel operation's payload with its client/server alias block.
Move Request above the Client/Server aliases and Transport interface that reference it, so types flow from least to most dependent.
Rename StreamClient/StreamServer to bare Client/Server in the iterator, relay, and writer packages (matching the deleter), keeping the ClientStream/ServerStream stream handle aliases. Updates the definitions and the gRPC/mock implementations.
Replace the explicit DeferCleanup(func() { Expect(pool.Close()).To(Succeed()) }) with
DeferClose at the OpenPool call site across the gRPC suite and transport tests, and
close the previously-unclosed pool in the construction test.
emilbon99
requested changes
Jun 26, 2026
emilbon99
left a comment
emilbon99
left a comment
Contributor
There was a problem hiding this comment.
Approved after comments addressed.
|
|
||
| // Pool is a pool of reusable gRPC client connections keyed by target address. Open one | ||
| // with [OpenPool] and acquire connections through the embedded [pool.Pool] interface. | ||
| type Pool struct { |
Contributor
There was a problem hiding this comment.
Do we even need this wrapper type?
Contributor
Author
There was a problem hiding this comment.
made it just a type alias but kept so type parameters didn't have to get passed around everywhere.
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
2 participants
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.
Issue Pull Request
Linear Issue
SY-4432
Description
Consolidates the distribution layer's node-to-node transport wiring into a single
distribution.Transportinterface that exposes bothChannel() channel.TransportandFramer() framer.Transport, replacing the previously separateChannelTransportandFrameTransportconfig fields.distribution.Transportinterface added inlayer.go.LayerConfignow takes oneTransportfield (validated/overridden as a unit) and wirescfg.Transport.Channel()/cfg.Transport.Framer()into the channel and framer services. All existing channel-domain configuration is preserved unchanged.transport/grpc/grpc.go): aTransportstruct that composes the existing per-domain gRPC channel and framer transports, withNew(pool),Channel(),Framer(), andBindableTransports()for server registration.transport/mock): the previously flat mock package is restructured intomock/channelandmock/framersubpackages (each exposing aNetworkthat provisions its domainTransport), plus a top-levelmock.Networkthat bundles them into a singledistribution.Transportper node. Retains shared in-memory network state across nodes.proxy/proxy.go):BatchFactory.Hostis now private; construct viaNewBatchFactory[T](host).Entry.Lease()now returnsnode.Key. All call sites adapted.cmd/startand the mock cluster builder adapted to construct the single bundled transport.The channel and framer transport interfaces, their proto definitions, and all channel-domain semantics (including
CreateMessage.Opts) are untouched — the unifiedTransportonly aggregates the existing per-domain transports.A new mock transport suite covers provisioning, channel create routing, framer delete routing, and shared network state through the bundled
distribution.Transport.Basic Readiness
Greptile Summary
This PR consolidates Synnax's distribution-layer node-to-node transport into a single
distribution.Transportinterface that bundlesChannel()andFramer()sub-transports, replacing the previously separateChannelTransportandFrameTransportfields inLayerConfig. It also restructures the mock transport package into dedicatedchannelandframersubpackages, adds a gRPC aggregator struct (transport/grpc), makesBatchFactory.Hostprivate via aNewBatchFactoryconstructor, and renamespool.New→pool.Open/factory.New→factory.Openfor consistency.LayerConfignow exposes a singleTransportfield; all channel and framer sub-transports are wired from it inOpenLayer. The gRPCTransportstruct aggregates per-domain gRPC implementations, and the mockNetworkmirrors this structure for in-memory tests.ChannelCreateService / ChannelDeleteService / ChannelRenameServicetoCreateService / DeleteService / RenameService, changing their full gRPC method paths on the wire.BatchFactory.Hostis now private (NewBatchFactoryconstructor required);pool.Factory.Newwas renamed toOpen; type aliases likeCreateTransportClientwere simplified toCreateClient. Middleware is now correctly applied to rename endpoints (fixing an omission in the old channel transport'sUsemethod).Confidence Score: 4/5
Safe to merge if all cluster nodes are always upgraded atomically; the gRPC service renames will silently break channel create/delete/rename RPCs in any mixed-version deployment.
The proto service renames change the full gRPC method paths on the wire (e.g., ChannelCreateService/Exec → CreateService/Exec). A node running the pre-PR binary will receive a gRPC Unimplemented error when dialing a post-PR node on those three channel endpoints, and vice versa. All other structural changes — transport unification, BatchFactory encapsulation, pool API rename, mock restructuring — are mechanically correct and well-tested.
core/pkg/distribution/channel/pb/services.proto and its generated files (services.pb.go, services_grpc.pb.go) contain the wire-breaking service renames; confirm that no mixed-version node communication is possible before merging.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD LC[LayerConfig.Transport\ndistribution.Transport] --> CH[Channel\nchannel.Transport] LC --> FR[Framer\nframer.Transport] CH --> CC[CreateClient / CreateServer] CH --> DC[DeleteClient / DeleteServer] CH --> RC[RenameClient / RenameServer] FR --> WR[Writer\nwriter.Transport] FR --> IT[Iterator\niterator.Transport] FR --> RL[Relay\nrelay.Transport] FR --> DL[Deleter\ndeleter.Transport] subgraph gRPC Implementation GT[grpc.Transport] --> GCH[grpc/channel.Transport] GT --> GFR[grpc/framer.Transport] end subgraph Mock Implementation MN[mock.Network] --> MCH[mock/channel.Network] MN --> MFR[mock/framer.Network] end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD LC[LayerConfig.Transport\ndistribution.Transport] --> CH[Channel\nchannel.Transport] LC --> FR[Framer\nframer.Transport] CH --> CC[CreateClient / CreateServer] CH --> DC[DeleteClient / DeleteServer] CH --> RC[RenameClient / RenameServer] FR --> WR[Writer\nwriter.Transport] FR --> IT[Iterator\niterator.Transport] FR --> RL[Relay\nrelay.Transport] FR --> DL[Deleter\ndeleter.Transport] subgraph gRPC Implementation GT[grpc.Transport] --> GCH[grpc/channel.Transport] GT --> GFR[grpc/framer.Transport] end subgraph Mock Implementation MN[mock.Network] --> MCH[mock/channel.Network] MN --> MFR[mock/framer.Network] endReviews (2): Last reviewed commit: "update network_test.go" | Re-trigger Greptile