gengorums: generate service clients by meling · Pull Request #379 · relab/gorums

meling · 2026-10-24T14:17:04Z

Closes #308. The generic terminal methods from the original issue are already on master (#378); this PR adds the generated service clients.
The design went through two revisions in review; the current one is the design update on #308: two client interfaces per service, one constructor each.

Important

This is a breaking change for users of the function-based API that v0.12.0 ships, so it targets v0.13.
Prepare the v0.13 release with gorums-release prepare -bump-min: the generated code no longer fits the v0.12 runtime, so MinVersion and GenVersion must become 13.
Without it, code generated by v0.12 passes EnforceVersion and then fails with undefined: gorums.ConfigContext.

Problem

Generated calls are package-level functions that take a *gorums.ConfigContext or a *gorums.NodeContext.
A caller must first wrap its context: pb.ReadQC(cfg.Context(ctx), req).
Go does not allow pb to declare methods on gorums.Config, so cfg.ReadQC(...) is not possible.
Package-level functions also block several services in one Go package (#349), because two services with the same RPC name collide.

Changes

For each service S, the generator emits up to two client interfaces, as gRPC does.
The call type in the proto file decides which interface a method goes to:

Interface	Constructor	Methods
`SClient`	`NewSClient(cfg gorums.Config)`	quorum calls, streaming quorum calls, multicasts, all on the configuration
`SNodeClient`	`NewSNodeClient(node *gorums.Node)`	remote calls and unicasts, all on one node

client := pb.NewStorageClient(cfg)
resp, err := client.ReadQC(ctx, req).Majority()
err = client.WriteMulticast(ctx, req).Send()

leader := pb.NewStorageNodeClient(cfg.Node(leaderID))
resp, err = leader.ReadRPC(ctx, req)
err = leader.WriteUnicast(ctx, req).Send()

Every call has the same shape, client.Method(ctx, req).
The target is fixed when the client is built.
A service with only one kind of call gets only that interface and constructor.
The constructors return the interfaces. The structs behind them are unexported (sClient, sNodeClient), and the doc comments are on the interface methods.
A struct name is the interface name with its leading acronym lowercased, for example tlsNodeClient for service TLS.
There are no helper methods: no Config(), WithConfig(), Node(), or ForNode().
An RPC may have any name.
Build node clients where you need them, and do not cache them.
A node client holds one pointer, so building one allocates nothing.
Building a configuration client allocates once, because the Config slice moves into an interface.
The static aliases are Config, ID, and Node, as before.

Runtime: the gorumsimpl constructors take (ctx, config, req, method) or (ctx, node, req, method).
ConfigContext, NodeContext, Config.Context, and Node.Context are removed.
A quorum call or multicast on an empty configuration panics when the call is built (see Return an error instead of panicking on an empty configuration #395 for a possible change).
A remote call or unicast on a nil node does not panic: it returns the new gorums.ErrNoNode, a gRPC status error with the InvalidArgument code.
So pb.NewStorageNodeClient(cfg.Node(id)) is safe for an ID that is not in cfg.
Generator: genClients generates each interface, constructor, and struct.
Each call-type template renders a method either as an interface declaration or as the struct method, so a signature is written once.
Name checks: a message, enum, enum value, or oneof wrapper type, also a nested one, must not take the name of an interface, constructor, or struct that the service generates.
Only the clients that a service has are reserved.
Migration: tests, examples, and benchkit use the clients.
benchkit's StartRemote, StopRemote, and the setup callbacks of MeasureLatency, ClientMeasured, and ServerMeasured now take (ctx context.Context, cfg gorums.Config) instead of a *gorums.ConfigContext.
Docs: user guide, dev guide, and a new "Function API to Service Clients" section in the migration guide.
Generated code, in the last commit of each group of changes.

Breaking changes

Generated package-level call functions are replaced by client interfaces.
gorums.ConfigContext, gorums.NodeContext, Config.Context, and Node.Context are removed, and so are the generated ConfigContext and NodeContext aliases.
The gorumsimpl constructor signatures change (generated code only).
A remote call or unicast on a nil node returns gorums.ErrNoNode instead of panicking.
The benchkit API listed above.
Hand-written code in a generated package that declares one of the generated client names, such as StorageClient or NewStorageNodeClient, no longer compiles.

Notes

Some intermediate commits do not pass all tests: a template commit comes before the commit that regenerates its output.
The tip passes.

Testing

Generator: golden files updated; failing-input cases for each generated name (interface, constructor, struct) against messages, enums, enum values, and oneof wrappers, also nested ones; golden cases for RPCs named Config, WithConfig, Node, and ForNode, and for a service with only node calls (nodeonly).
TestGeneratedCodeClient: node calls through a node client reach the given node, also a node outside the client's configuration; a nil node reports ErrNoNode; an empty configuration panics.
TestGeneratedCodeClientAllocs: one allocation for a configuration client, none for a node client.
TestCallNilNode and TestCallConstructorsPanic in internal/impl.
go test ./... in the root, examples, and benchkit modules, with and without -tags=integration; make test, make goplscheck, make modernize, and golangci-lint are clean.
Benchmarks (QuorumCall, count 6, benchstat) were measured on an earlier version of this PR, before the clients became interfaces. They were not run again.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

meling · 2026-10-26T21:22:47Z

[P3] Include nested protobuf types in the client-name check

The callers of this check visit only the top-level f.Messages and f.Enums, but nested protobuf types also declare package-level Go names. For example, message Outer { message InnerClient {} } together with service Outer_Inner { rpc Read(Request) returns (Response) {} } passes generation and then fails to compile with Outer_InnerClient redeclared: both protoc-gen-go and the new client template declare that type. I reproduced this with gentest.Generate followed by gentest.Compile; the same schema compiles at the PR base (a608bb06) and still fails at #380's tip. Recursively check each message's nested messages and enums by GoIdent.GoName so these collisions receive the intended generator error.

Location: cmd/protoc-gen-gorums/gengorums/generate.go:113.

Validation: the existing test suites pass in the root, examples, and benchkit modules.

The generated service clients for #308 hold their configuration or node and call the runtime with a plain context.Context. The ConfigContext and NodeContext wrappers no longer carry their weight, so the call constructors now take the context and the target as separate arguments, and the wrapper types and the Config.Context and Node.Context methods are removed. The empty-configuration and nil-node checks move from the removed Context methods into the constructors, and the panic names the constructor that was called. Generated code still uses the old signatures; later commits update the generator and regenerate it.

The runtime constructors now take the context and the target directly, so the generated package-level call functions that took a ConfigContext or NodeContext no longer compile. Generate, for each service S, an SClient bound to a gorums.Config and an SClientNode bound to a *gorums.Node, and generate each call as a method on the client that matches its target: quorum calls, streaming quorum calls, and multicasts on SClient; remote calls and unicasts on SClientNode. The client types live in their own service-level template, so dev mode writes them to zorums_client_gorums.pb.go. The ConfigContext and NodeContext aliases are gone from the static code, which also removes them from the reserved identifiers.

Each service now generates SClient and SClientNode types with helper methods, so an RPC method collides only with the helpers of the client type that receives it: Config, WithConfig, and ForNode on SClient, and Node on SClientNode. A remote call named Config is therefore valid, and the generator no longer checks RPC names against the type aliases. The client types and their constructors are package-level names, so messages and enums in the Go package must not use them either. Option validation now runs before the name checks, because the call type of a method selects its receiver.

The generated package-level call functions and the ConfigContext and NodeContext types are gone, so every caller now builds a generated service client from its configuration or node and passes the context to each call. Nested calls in server handlers build the client from the handler's peer configuration and pass the handler's context. The benchkit helpers StartRemote and StopRemote take a context and a configuration instead of a ConfigContext, and the setup callbacks of MeasureLatency, ClientMeasured, and ServerMeasured receive the run's context and configuration, so a benchmark builds its own client. New dev tests check the client target accessors, that ForNode and NewZorumsServiceClientNode reach the same node, including a node outside the client's configuration, and that constructing and converting clients allocates nothing.

The call functions that took a ConfigContext or NodeContext are gone; each service now has SClient and SClientNode types whose methods take a context.Context. Update the user guide, the developer guide, and the package docs to show the client types, and add a migration section for code written against the function API.

Regenerate with make dev and make genproto for the generated service clients.

The generated doc comments of unicast and multicast methods named the service and method, which the context already shows. They also folded Send and Async into one long sentence. State the call target, then say how Send and Async dispatch the request and where each reports the send error, in short sentences.

The name check visited only top-level messages and enums. But protoc-gen-go declares nested types at package level, with names such as Outer_InnerClient. A nested type could thus take the name of a generated client type for a service such as Outer_Inner. Generation then succeeded and the package failed to compile with a redeclared type. Walk nested messages and enums recursively, so such a schema gets the generator's error instead.

The doc comment said asyncQCFunc dispatches one quorum call "with client", which does not say what client does. Name the QuorumCall method of the BenchmarkClient and the nodes it calls.

The service-client migration reworded these doc comments to say that cfg holds the remote servers. The possessive wording from before reads better, and cfg needs no extra explanation. Restore the original text; only the signatures changed.

MeasureLatency, ClientMeasured, and ServerMeasured each spelled out the same setup function type inline, with parameter names, which made the signatures long. Declare it once as a named type. It is exported, because these exported functions take it as a parameter, so callers and godoc need to see it. SetupFunc names its role: it runs once per configuration, outside the measured window, and builds the operation that the harness then times.

The doc comments of MeasureLatency, ClientMeasured, and ServerMeasured walked through the lifecycle step by step. State only what each function takes, what it returns, and what it measures. The steps stay visible in the code and its inline comments.

The benchkit package has a generated Config alias for gorums.Config. Use it in EstimateOffsets, and drop the gorums import that this file then no longer needs.

Regenerate with the clarified unicast and multicast doc comments.

meling · 2026-10-26T23:29:02Z

Re: nested protobuf types in the client-name check (comment above). Fixed in ae99a61: the check now walks nested messages and enums recursively by GoIdent.GoName. New failing cases reproduce message Outer { message InnerClient {} } with service Outer_Inner, and a two-level nested enum named after a generated ClientNode identifier; both now fail generation with the generator's error instead of a compile error. The dev guide and migration guide mention nested types.

Each service had two client types: SClient for configuration calls and SClientNode for node calls, with ForNode, NewSClientNode, and Node to move between them. Generate only SClient. Remote calls and unicasts become methods on SClient that take the target node after the context, as the runtime constructors do. The method signature still keeps node calls and configuration calls apart at compile time, and the node can be any node, also one outside the client's configuration. A node call does not use the client's configuration, so the zero SClient is ready for node calls; the type's doc comment says so. The helper methods are now Config and WithConfig on one receiver, so an RPC with either name collides whatever its call type, and Node and ForNode are legal RPC names.

The generated SClientNode type is gone; remote calls and unicasts are methods on SClient that take the node after the context. Migrate the tests and the storage example: client.ForNode(n).M(ctx, in) and NewSClientNode(n).M(ctx, in) become client.M(ctx, n, in), or SClient{}.M(ctx, n, in) where no configuration is at hand. The generated-code tests drop the ForNode, NewSClientNode, and Node checks and their allocation checks. They now show that a zero client makes node calls, that a node outside the client's configuration works, and that a quorum call on a zero client panics.

WithConfig ignored its receiver and returned the same client as NewSClient. Remove it, so that a client for another configuration comes from NewSClient alone. An RPC method may now be named WithConfig; only Config remains a reserved method name.

A node call ignores the client's configuration, so its doc comment example now calls it on the zero client, qualified by the generated package name. The comment also says why a node call is a client method: the client type keeps the service's calls together and avoids package-level name conflicts. The one-way call comments describe Send and Async in separate sentences, with Wait on the handle that Async returns.

A call on an empty configuration or a nil node panics. The panic message now names the full method name of the call next to its call type, so the message points at the failing call site.

The test protos under internal/testprotos use the internal.testprotos proto package. The clientnames proto and the inline test protos used gorums.testprotos instead.

A node call does not use the client's configuration. The client type keeps the service's calls together and avoids package-level name conflicts, so the zero client is ready for node calls. Say this in the package doc, the user guide, and the developer guide. Describe Send and Async in separate sentences, with Wait on the handle that Async returns. Say what the Call handle provides instead of saying that it adds Intercept. Split long sentences, and use execute and remote call in the package doc.

Match the package doc, which says that a call executes on a configuration or a node.

A node client may be built from the nil node that Config.Node returns for an unknown ID. A call on that client should fail like a call to a node that is down, not panic. RemoteCall now returns ErrNoNode, and Unicast returns a handle whose Send and Wait report ErrNoNode without sending. ErrNoNode is a gRPC status error with the Unavailable code, like ErrNodeClosed and ErrStreamDown. Quorum calls and multicasts still panic on an empty configuration.

The single client type made node calls take a node argument and look different from configuration calls, and its zero value needed its own documentation. Each service now gets up to two client interfaces, as in gRPC: SClient with the quorum calls and multicasts, built from a configuration by NewSClient, and SNodeClient with the remote calls and unicasts, built from a node by NewSNodeClient. Every call has the shape Method(ctx, in). A service with only one kind of call gets only that client. Unexported structs implement the interfaces. Each call type template renders either the documented interface method or the implementing method, so the signature is written once. In dev mode, the client file holds the interfaces, constructors, and structs, and each call type file holds the implementing methods. The clients have no helper methods, so an RPC may be named Config. The collision check now covers the names of the generated interfaces, constructors, and structs, for the clients that a service has.

The generated node calls moved from SClient, where they took the node as an argument, to SNodeClient, which is built from the node. Tests and the storage example now call NewSNodeClient(node).Method(ctx, in), and the generated code tests use the caller's configuration instead of the removed Config method. The generated code tests also check that a node client of a nil node reports ErrNoNode, that a quorum call on an empty configuration still panics, and that building a node client does not allocate.

Replace the one-client-type text, including the zero-value client and the namespace explanation, with the two client interfaces. The docs now state the rules for their use: build node clients where they are needed and do not keep them, a nil node returns ErrNoNode, any node works, an empty configuration panics, and a new configuration needs a new SClient. The dev guide describes the two rendering modes of the call type templates and the extended name checks.

meling · 2026-10-28T22:35:10Z

Pushed five new commits that replace the one-client-type design with two client interfaces per service.
The design and its rules are in the #308 design update.

SClient has the quorum calls and multicasts; NewSClient(cfg) builds it.
SNodeClient has the remote calls and unicasts; NewSNodeClient(node) builds it. Build it where you need it; do not cache it.
Every call is client.Method(ctx, req). The generated clients have no Config(), WithConfig(), Node(), or ForNode().
A nil node does not panic; every call returns the new gorums.ErrNoNode.
Building a configuration client allocates once; building a node client allocates nothing.

The first four commits do not pass all tests alone; the last commit regenerates the generated files.
The thread about the client type (4383000477) is left open for you.

A nil node is a caller error that retrying cannot fix, unlike the transient conditions that the Unavailable code describes.

A service named TLS now gets the struct tlsNodeClient, not tLSNodeClient.

Test .Decl negatively in the method body templates, and separate the synchronous and asynchronous examples of one-way calls. Document the two rendering modes where renderMethod is defined.

meling

Reviewed the current tip, ef8e0df, against master (723472a).
Two generator name collisions are reproduced below: the oneof-wrapper collision also persists at #380's current tip; the client/Correctable-alias collision is already rejected by #380 and needs its guard moved into this prerequisite PR.

Validation: go test ./... passes in the root, examples, and benchkit modules.
Both reproducer schemas compile at master; the inline comments describe their results at the two PR tips.
The earlier nested-message, enum-value, import-shadowing, and nil-node fixes are present.

The PR description still describes the superseded single-client design, including node arguments, a zero-value client, Config(), zero allocations, and a panic for a nil node.
Please update it to the current SClient/SNodeClient interfaces and ErrNoNode behavior.

protoc-gen-go declares a type at package level for each field of a oneof, named Message_Field, with a lowercase first letter in the opaque API. Such a type can take the name of a generated client interface or struct.

meling mentioned this pull request Oct 24, 2026

gengorums: support several services per proto file and Go package #380

Merged

meling force-pushed the feature/308/service-clients branch from c4907b3 to f473a59 Compare October 25, 2026 09:26

Base automatically changed from feature/316/generator-test-harness to master October 25, 2026 09:37

meling force-pushed the feature/308/service-clients branch from f473a59 to 5779ceb Compare October 25, 2026 20:57

meling mentioned this pull request Oct 25, 2026

Generated service clients #308

Closed

meling marked this pull request as ready for review October 25, 2026 23:35

Copilot AI lite review requested due to automatic review settings October 25, 2026 23:35

Copilot AI reviewed Oct 25, 2026

meling force-pushed the feature/308/service-clients branch from 5779ceb to 763d279 Compare October 25, 2026 23:51

meling added this pull request to stack #391 October 25, 2026 23:51

meling commented Oct 26, 2026

View reviewed changes

meling added 14 commits October 26, 2026 23:59

all: regenerate Gorums code for service clients

8f1882f

Regenerate with make dev and make genproto for the generated service clients.

benchkit/benchmark: say which call asyncQCFunc dispatches

940378f

The doc comment said asyncQCFunc dispatches one quorum call "with client", which does not say what client does. Name the QuorumCall method of the BenchmarkClient and the nodes it calls.

benchkit: use the generated Config alias in EstimateOffsets

d2b9074

The benchkit package has a generated Config alias for gorums.Config. Use it in EstimateOffsets, and drop the gorums import that this file then no longer needs.

all: regenerate Gorums code

4988d84

Regenerate with the clarified unicast and multicast doc comments.

meling force-pushed the feature/308/service-clients branch from 763d279 to 4988d84 Compare October 26, 2026 23:27

meling added 2 commits October 27, 2026 00:45

meling added 13 commits October 28, 2026 00:26

dev: use the dev Config and Node aliases in the generated code tests

63ea845

gorums: name the method in call construction panics

574640a

A call on an empty configuration or a nil node panics. The panic message now names the full method name of the call next to its call type, so the message points at the failing call site.

testprotos: use the internal.testprotos proto package

05ba868

The test protos under internal/testprotos use the internal.testprotos proto package. The clientnames proto and the inline test protos used gorums.testprotos instead.

gengorums: say execute in the client type comment

3096708

Match the package doc, which says that a call executes on a configuration or a node.

all: regenerate Gorums code

a2d6ee9

all: regenerate Gorums code

f3e334b

meling commented Oct 29, 2026

View reviewed changes

meling added 8 commits October 29, 2026 23:25

gorums: report ErrNoNode with the InvalidArgument code

b3f910f

A nil node is a caller error that retrying cannot fix, unlike the transient conditions that the Unavailable code describes.

impl: build the empty configuration panic message with Sprintf

1fbb6cc

gengorums: lower a leading acronym in unexported client names

8b407fb

A service named TLS now gets the struct tlsNodeClient, not tLSNodeClient.

gengorums: skip the else branch in method bodies and split examples

42db2f9

Test .Decl negatively in the method body templates, and separate the synchronous and asynchronous examples of one-way calls. Document the two rendering modes where renderMethod is defined.

examples/storage: drop the unused receiver of the node call helpers

dd27d12

doc: keep only the high-level client generation in the dev guide

6071bba

all: regenerate Gorums code

6978c7a

doc: give the InvalidArgument code of ErrNoNode

ef8e0df

meling mentioned this pull request Oct 29, 2026

Return an error instead of panicking on an empty configuration #395

Closed

meling commented Oct 29, 2026

View reviewed changes

Comment thread cmd/protoc-gen-gorums/gengorums/generate.go Resolved

Comment thread cmd/protoc-gen-gorums/gengorums/generate.go Resolved

meling added 2 commits October 30, 2026 00:16

gengorums: check oneof wrapper names for client name collisions

22d5ffc

protoc-gen-go declares a type at package level for each field of a oneof, named Message_Field, with a lowercase first letter in the opaque API. Such a type can take the name of a generated client interface or struct.

doc: list oneof wrapper types among the names a client reserves

e3485ab

meling merged commit 889dcba into master Oct 30, 2026
3 checks passed

meling deleted the feature/308/service-clients branch October 30, 2026 19:00

meling mentioned this pull request Oct 30, 2026

doc: add a migration guide from v0.11 to v0.13 #397

Open

Conversation

meling commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Breaking changes

Notes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

meling commented Oct 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

meling commented Oct 26, 2026

Uh oh!

meling commented Oct 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

meling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

meling commented Oct 24, 2026 •

edited

Loading