XnLemon commented on code in PR #1017:
URL: https://github.com/apache/dubbo-go-pixiu/pull/1017#discussion_r3735250761
##########
pkg/filter/http/grpcproxy/descriptor.go:
##########
@@ -117,12 +130,103 @@ func (dr *Descriptor)
getServerDescriptorSourceCtx(refCtx context.Context, cfg *
default:
err = errors.Errorf("found a value of type %s, which is not
*grpc.ClientConn, ", t)
}
- return &serverSource{client: grpcreflect.NewClient(refCtx,
reflectpb.NewServerReflectionClient(cc))}, err
+ if err != nil {
+ return nil, err
+ }
+
+ dr.remoteMu.Lock()
+ defer dr.remoteMu.Unlock()
+ if dr.remoteSources == nil {
+ dr.remoteSources = make(map[*grpc.ClientConn]DescriptorSource)
+ }
+ if source, ok := dr.remoteSources[cc]; ok {
+ return source, nil
+ }
+
+ // The descriptor source is cached for the lifetime of the connection.
It
+ // must not retain the per-request context, which is cancelled when
Decode
+ // returns and would make every later reflection call fail.
+ source := &serverSource{client:
grpcreflect.NewClient(context.Background(),
reflectpb.NewServerReflectionClient(cc))}
Review Comment:
[P1] Reflection 丢失请求超时
用 context.Background() 创建缓存的 reflection client。后续 FindSymbol 不再受 c.Timeout
控制;如果后端已连接但 reflection 卡住,请求可以无限挂起。原实现使用 request context,不存在这个回归。缓存可以长期存在,但每次远程
lookup 仍必须有明确 deadline。
该PR 将 reflection RPC 从 request-scoped context 改成了 lifetime-scoped
Background,直接改变了超时行为。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]