gorums: separate unary and streaming server handler types

クローズ
#435 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

関連するプルリクエストがすでにマージされています。

  • #454 @meling による — マージ済み

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
15/100
issue の種類
リファクタリング
明瞭さ
おおむね明確
活発さ
活発
技術スタック
go
領域
api

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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?
主要言語
Go
スター
149
フォーク
15
平均マージ
1日 3時間
マージ済み PR(30日)
52

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

relab/gorums のほかの issue

relab/gorums の issue をすべて見る

似ている issue

Go の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。