Similarityoung commented on code in PR #1017:
URL: https://github.com/apache/dubbo-go-pixiu/pull/1017#discussion_r3743969109


##########
pkg/server/cluster_manager.go:
##########
@@ -142,10 +165,27 @@ func (cm *ClusterManager) SetEndpoint(clusterName string, 
endpoint *model.Endpoi
 
 func (cm *ClusterManager) DeleteEndpoint(clusterName string, endpointID 
string) {
        cm.rw.Lock()
-       defer cm.rw.Unlock()
-
+       var endpointAddress string
+       if clusterConfig := cm.store.findClusterConfig(clusterName); 
clusterConfig != nil {
+               for _, endpoint := range clusterConfig.Endpoints {
+                       if endpoint != nil && endpoint.ID == endpointID {
+                               endpointAddress = endpoint.Address.GetAddress()
+                               break
+                       }
+               }
+       }
        cm.store.IncreaseVersion()
        cm.store.DeleteEndpoint(clusterName, endpointID)
+       handlers := make([]func(string, string), 0, 
len(cm.endpointRemovalHandlers))
+       for _, handler := range cm.endpointRemovalHandlers {
+               handlers = append(handlers, handler)
+       }
+       cm.rw.Unlock()
+       if endpointAddress != "" {
+               for _, handler := range handlers {
+                       handler(clusterName, endpointAddress)
+               }

Review Comment:
   `SetEndpoint` 在同一实例 ID 移动到新地址时会直接替换旧端点,但新增的移除回调只在 `DeleteEndpoint` 中触发。 因此 
gRPC 管理器会一直保留旧的 `grpc.ClientConn` 和描述符缓存;注册中心反复更新实例地址时,连接、goroutine 
和文件描述符会持续累积,直到过滤器整体关闭。 请在端点地址替换、整库替换和集群替换时也通知并清理被移除的旧地址,而不应只处理显式删除。



##########
pkg/filter/http/grpcproxy/descriptor.go:
##########
@@ -117,12 +132,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
+       }
+
+       // 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.generation++

Review Comment:
   `removeConnection` 会递增全局代次,因此删除端点 B 会使无关且健康的端点 A 正在执行的首次描述符查询失效。 `Decode` 
随后把 `descriptor cache invalidated` 映射为 HTTP 405,使合法请求在反射查询与其他端点删除并发时失败。 
请按连接维护失效状态,仅在过滤器整体关闭时使用全局代次,避免影响无关连接的查询。



-- 
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]

Reply via email to