AlexStocks commented on code in PR #1017:
URL: https://github.com/apache/dubbo-go-pixiu/pull/1017#discussion_r3802791664
##########
pkg/filter/http/grpcproxy/descriptor.go:
##########
@@ -117,12 +128,79 @@ 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
+ }
+
+ // The reflection client is created per lookup and bound to the request
+ // context so every remote reflection RPC honors the request timeout.
+ // It must not be cached connection-scoped: grpcreflect reuses the root
+ // context for every RPC, and a cached client would lose the deadline
and
+ // keep the per-request timeout from applying. The method descriptor
+ // cache in getMethodDescriptor below is what avoids repeating the
+ // reflection RPC after the first lookup.
+ return &serverSource{client: grpcreflect.NewClientV1Alpha(refCtx,
reflectpb.NewServerReflectionClient(cc))}, nil
}
// nolint
func (dr *Descriptor) getServerDescriptorSource(refCtx context.Context, cc
*grpc.ClientConn) DescriptorSource {
- return &serverSource{client: grpcreflect.NewClient(refCtx,
reflectpb.NewServerReflectionClient(cc))}
+ return &serverSource{client: grpcreflect.NewClientV1Alpha(refCtx,
reflectpb.NewServerReflectionClient(cc))}
+}
+
+func (dr *Descriptor) removeConnection(cc *grpc.ClientConn) {
+ if cc == nil {
+ return
+ }
+ dr.methodMu.Lock()
+ delete(dr.methodDescs, cc)
+ dr.methodMu.Unlock()
+}
+
+func (dr *Descriptor) Close() {
+ dr.methodMu.Lock()
+ dr.methodDescs = nil
+ dr.methodMu.Unlock()
+}
+
+func (dr *Descriptor) getMethodDescriptor(source DescriptorSource, cc
*grpc.ClientConn, service, method string) (*desc.MethodDescriptor, error) {
+ key := service + "\x00" + method
+ dr.methodMu.RLock()
+ if methods := dr.methodDescs[cc]; methods != nil {
+ if descriptor, ok := methods[key]; ok {
+ dr.methodMu.RUnlock()
+ return descriptor, nil
+ }
+ }
+ dr.methodMu.RUnlock()
+
+ dr.methodMu.Lock()
+ defer dr.methodMu.Unlock()
+ if methods := dr.methodDescs[cc]; methods != nil {
+ if descriptor, ok := methods[key]; ok {
+ return descriptor, nil
+ }
+ }
+
+ dscp, err := source.FindSymbol(service)
Review Comment:
[P1] 把网络查询移出全局锁后，当前 singleflight 又把同一 cache miss 的所有等待者绑定到了第一个请求的
DescriptorSource；REMOTE/AUTO 的 source 捕获该请求的 context。判别探针让首个 source 返回 context
deadline exceeded，同时第二个调用使用可立即成功的 source；因为 loadKey
相同，第二个调用仍继承首个错误。于是一个短超时或主动取消的请求会让仍在有效 deadline 内的并发请求一起失败，Decode 还会把它们映射成 HTTP
405。请让共享 lookup 使用不归属于任一等待者的受控 context，并让每个调用者独立等待/取消，或不要跨不同 request context
合并这次网络查询。
--
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]
