gorums: separate unary and streaming server handler types
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 15/100
Research direction
This is an open design proposal, so decide the open questions before any code. Start with the current Handler and ServerInterceptor types, RegisterHandler, and HandleRequest, which the issue names. The proposal also changes the generator's server template and about 36 hand-written handlers in tests and gorumstest. Done means a maintainer has settled the naming and the unary send behaviour, and the issue has a linked plan.
Written by the indexing model from the issue text.
Description
Summary
Every gorums.Handler takes a send func(proto.Message) callback, but only streaming handlers use it.
Unary and one-way handlers ignore it with _.
This issue proposes two handler types: one for unary and one-way calls without send, and one for streaming calls with send.
Interceptors keep a single type that covers all call types.
This follows #424 and #431, which made handlers return a proto.Message and moved send from ServerContext to a handler argument.
The proposal is not urgent.
It is worth doing only if hand-written handlers become a common part of the API, for example for back-channel calls.
Current API (after #431)
type Handler func(ServerContext, *Message, func(proto.Message)) (proto.Message, error)
type ServerInterceptor func(ServerContext, *Message, func(proto.Message), Handler) (proto.Message, error)
func (s *Server) RegisterHandler(method string, handler Handler)
How a handler delivers its responses depends on the call type:
- Unary: it returns the response.
- Streaming: it calls
sendonce for each response, and returnsnil, nil. - One-way: it returns
nil, nil, and the server sends no response.
Proposal
// Handler handles a unary or one-way call.
type Handler func(ServerContext, *Message) (proto.Message, error)
// StreamHandler handles a streaming call. It calls send once for each response.
type StreamHandler func(ServerContext, *Message, func(proto.Message)) (proto.Message, error)
// ServerInterceptor wraps every call type, so next takes send.
type ServerInterceptor func(ServerContext, *Message, func(proto.Message), StreamHandler) (proto.Message, error)
func (s *Server) RegisterHandler(method string, handler Handler)
func (s *Server) RegisterStreamHandler(method string, handler StreamHandler)
RegisterHandler converts a Handler to the general form once, at registration:
func (s *Server) RegisterHandler(method string, handler Handler) {
general := func(ctx ServerContext, in *Message, _ func(proto.Message)) (proto.Message, error) {
return handler(ctx, in)
}
s.handlers[method] = entry{handler: chainInterceptors(general, s.interceptors...)}
}
The server records which methods stream.
For a unary or one-way method, HandleRequest passes a shared package-level send function that does nothing (or panics), so it allocates no callback.
Examples
Hand-written unary handler
Before:
srv.RegisterHandler(mock.TestMethod, func(_ gorums.ServerContext, in *gorums.Message, _ func(proto.Message)) (proto.Message, error) {
return gorums.AsProto[*pb.StringValue](in), nil
})
After:
srv.RegisterHandler(mock.TestMethod, func(_ gorums.ServerContext, in *gorums.Message) (proto.Message, error) {
return gorums.AsProto[*pb.StringValue](in), nil
})
Hand-written streaming handler
srv.RegisterStreamHandler(mock.StreamMethod, func(_ gorums.ServerContext, in *gorums.Message, send func(proto.Message)) (proto.Message, error) {
for i := range 3 {
send(pb.String(fmt.Sprintf("%s-%d", gorums.AsProto[*pb.StringValue](in).GetValue(), i)))
}
return nil, nil
})
Generated registration
The template picks the register method by call type:
srv.RegisterHandler("pkg.Service.Read", func(ctx gorums.ServerContext, in *gorums.Message) (proto.Message, error) {
req := gorums.AsProto[*ReadRequest](in)
return impl.Read(ctx, req)
})
srv.RegisterStreamHandler("pkg.Service.Watch", func(ctx gorums.ServerContext, in *gorums.Message, send func(proto.Message)) (proto.Message, error) {
req := gorums.AsProto[*WatchRequest](in)
impl.Watch(ctx, req, func(resp *WatchResponse) { send(resp) })
return nil, nil
})
Interceptor
Interceptors do not change, except for the type of next:
func StreamLoggingInterceptor(ctx gorums.ServerContext, in *gorums.Message, send func(proto.Message), next gorums.StreamHandler) (proto.Message, error) {
logAndSend := func(resp proto.Message) {
log.Printf("Method=%s, Message=%v", in.GetMethod(), resp)
send(resp)
}
return next(ctx, in, logAndSend)
}
Gains
- Simpler unary handlers. Hand-written unary handlers lose the
_ func(proto.Message)parameter.
The repository has 36 such handlers in tests andgorumstest, and 3 streaming handlers.
Back-channel handlers in applications are mostly unary. - Type safety. A unary handler cannot call
send.
Today a unary handler can callsendand also return a response, so the server replies twice to one call. - One allocation less for each unary and one-way request.
HandleRequestbuilds thesendclosure only for streaming methods.
In aHandleRequestmicrobenchmark, the unary path drops from 7 to 6 allocations and from 310 to 286 bytes per request.
Costs
- More API. One more handler type and one more register method.
- Interceptors still need the general form. One interceptor chain serves all call types, so
nextmust acceptsend.
The general form therefore stays public, asStreamHandlerin the sketch above.
The name is awkward, because an interceptor callsnextfor unary calls too.
A neutral name, such asCallHandler, makes three handler-like types. - Handler kind must match the method.
RegisterHandleron a streaming method gives a handler that cannot send.
Generated code always picks correctly, but hand-written code can make this mistake, and the server cannot detect it at compile time. - Generator change. The server template chooses the register method by call type.
Code generated by an earlier release no longer compiles, so the next release needs-bump-min.
Alternatives
- Keep one
Handler(current). One handler type, one interceptor type, and one register method.
Generated code hides the unused_; only hand-written handlers see it. - Split like gRPC. Two handler types and two interceptor types (
UnaryServerInterceptor,StreamServerInterceptor).
Each type is clean, but a cross-cutting interceptor, such as logging or delay, must be written and registered twice. - Put
SendonServerContext. Every handler could call it again, which reverses what #424 set out to do.
Open questions
- Is the gain worth one more public handler type, given that interceptors still see
send? - If yes, what is the name of the general form that interceptors receive as
next? - Should the shared
sendfor unary methods do nothing, or panic to expose a handler of the wrong kind?
- Dominant language
- Go
- Stars
- 149
- Forks
- 15
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 50
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from relab/gorums
-
cleanup performance
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
doc
Difficulty 3/5 Half a day Newbie friendliness 45/100
Maintainers usually reply within 1 day
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
enhancement
Difficulty 5/5 Over a week Newbie friendliness 30/100
Maintainers usually reply within 1 day
-
doc
Difficulty 3/5 1-2 days Newbie friendliness 65/100
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
dragonflydb/dragonfly-operator#619 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 67/100
cloudfoundry/bosh-bootloader#701 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
baremetal: private network check rejects offer-specific options (Private Network 10Gbps / 25Gbps)Possibly taken @lbarrosby claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
scaleway/terraform-provider-scaleway#4587 ·
Maintainers usually reply within 1 day
-
blocked-needs-validation bug
Difficulty 1/5 Under an hour Newbie friendliness 78/100
projectcapsule/capsule#2280 ·
Maintainers usually reply within 1 day