Skip to content

codegen: create RPC service interface with gRPC & DRPC adapters #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

shubhamdhama
Copy link

stacked on #2
Define a shared RPC<Service>Client interface to wrap both gRPC's
<Service>Client and DRPC's DRPC<Service>Client. Streaming methods will
also return the common RPC<Service>_<Method>Client.

Adds two simple adapters:

  • NewGRPC<Service>ClientAdapter
  • NewDRPC<Service>ClientAdapter

They implement RPC<Service>Client, so we can swap clients at runtime via
the cluster setting.

shubhamdhama added a commit to shubhamdhama/cockroach that referenced this pull request May 18, 2025
Enable MultiRaft service on the DRPC server in addition to gRPC. Controlled
by `rpc.experimental_drpc.enabled` (off by default).

It uses the adapters introduced by:
cockroachdb/drpc#4
@shubhamdhama shubhamdhama force-pushed the add-grpc-drpc-adapters branch from 2168317 to 23a74f1 Compare May 28, 2025 06:35
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this pull request May 30, 2025
Enable MultiRaft service on the DRPC server in addition to gRPC. Controlled
by `rpc.experimental_drpc.enabled` (off by default).

It uses the adapters introduced by:
cockroachdb/drpc#4
@shubhamdhama
Copy link
Author

shubhamdhama commented Jun 2, 2025

Example code generated by this:

// MultiRaft gRPC -> RPC adapter
type grpcMultiRaftClientAdapter multiRaftClient

func NewGRPCMultiRaftClientAdapter(conn *grpc.ClientConn) RPCMultiRaftClient {
	return (*grpcMultiRaftClientAdapter)(&multiRaftClient{conn})
}

func (a *grpcMultiRaftClientAdapter) RaftMessageBatch(ctx context.Context) (RPCMultiRaft_RaftMessageBatchClient, error) {
	return (*multiRaftClient)(a).RaftMessageBatch(ctx)
}

func (a *grpcMultiRaftClientAdapter) RaftSnapshot(ctx context.Context) (RPCMultiRaft_RaftSnapshotClient, error) {
	return (*multiRaftClient)(a).RaftSnapshot(ctx)
}

func (a *grpcMultiRaftClientAdapter) DelegateRaftSnapshot(ctx context.Context) (RPCMultiRaft_DelegateRaftSnapshotClient, error) {
	return (*multiRaftClient)(a).DelegateRaftSnapshot(ctx)
}

// compile-time assertion
var _ RPCMultiRaftClient = (*grpcMultiRaftClientAdapter)(nil)

// MultiRaft DRPC -> RPC adapter
type drpcMultiRaftClientAdapter drpcMultiRaftClient

func NewDRPCMultiRaftClientAdapter(conn drpc.Conn) RPCMultiRaftClient {
	return (*drpcMultiRaftClientAdapter)(&drpcMultiRaftClient{conn})
}

func (a *drpcMultiRaftClientAdapter) RaftMessageBatch(ctx context.Context) (RPCMultiRaft_RaftMessageBatchClient, error) {
	return (*drpcMultiRaftClient)(a).RaftMessageBatch(ctx)
}

func (a *drpcMultiRaftClientAdapter) RaftSnapshot(ctx context.Context) (RPCMultiRaft_RaftSnapshotClient, error) {
	return (*drpcMultiRaftClient)(a).RaftSnapshot(ctx)
}

func (a *drpcMultiRaftClientAdapter) DelegateRaftSnapshot(ctx context.Context) (RPCMultiRaft_DelegateRaftSnapshotClient, error) {
	return (*drpcMultiRaftClient)(a).DelegateRaftSnapshot(ctx)
}

// compile-time assertion
var _ RPCMultiRaftClient = (*drpcMultiRaftClientAdapter)(nil)

Define a shared `RPC<Service>Client` interface to wrap both gRPC's
`<Service>Client` and DRPC's `DRPC<Service>Client`. Streaming methods will
also return the common `RPC<Service>_<Method>Client`.

Adds two simple adapters:
- `NewGRPC<Service>ClientAdapter`
- `NewDRPC<Service>ClientAdapter`

They implement `RPC<Service>Client`, so we can swap clients at runtime via
the cluster setting.
@shubhamdhama shubhamdhama force-pushed the add-grpc-drpc-adapters branch from 23a74f1 to c5c11ef Compare June 2, 2025 09:02
@shubhamdhama
Copy link
Author

Same as #2 (comment) here for why tests are missing for these changes.

Copy link

@cthumuluru-crdb cthumuluru-crdb left a comment

Choose a reason for hiding this comment

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

LGTM!

@shubhamdhama shubhamdhama merged commit 5b0c5d2 into cockroachdb:main Jun 3, 2025
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this pull request Jun 3, 2025
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