gorums: separate unary and streaming server handler types

Open
#435 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
15/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
go
Domain
api

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 send once for each response, and returns nil, 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 and gorumstest, 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 call send and also return a response, so the server replies twice to one call.
  • One allocation less for each unary and one-way request. HandleRequest builds the send closure only for streaming methods.
    In a HandleRequest microbenchmark, 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 next must accept send.
    The general form therefore stays public, as StreamHandler in the sketch above.
    The name is awkward, because an interceptor calls next for unary calls too.
    A neutral name, such as CallHandler, makes three handler-like types.
  • Handler kind must match the method. RegisterHandler on 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 Send on ServerContext. 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 send for 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from relab/gorums

All issues in relab/gorums

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.