AlexStocks commented on code in PR #3579:
URL: https://github.com/apache/dubbo-go/pull/3579#discussion_r3687568764


##########
protocol/invocation/rpcinvocation.go:
##########
@@ -244,7 +266,7 @@ func (r *RPCInvocation) GetAttributeWithDefaultValue(key 
string, defaultValue an
 }
 
 func (r *RPCInvocation) GetAttachmentAsContext() context.Context {
-       ctx := context.Background()
+       ctx := r.Context()

Review Comment:
   [P1] 保留 context 中已有的 Triple outgoing attachments
   
   从存储的 request context 开始仍不足以保留其 outgoing metadata:本方法最后调用的 
`triple_protocol.NewOutgoingContext` 明确会替换已有 outgoing headers。exact Head 探针给 
context 设置 `Existing` header,再加入一个 invocation attachment,结果 `Existing` 消失而新增 
attachment 保留;脚本路由调用本方法再 merge 时会静默删除调用方 metadata。请先 clone 
`ExtractFromOutgoingContext(r.Context())`,按明确优先级覆盖 invocation attachments 后再创建 
outgoing context,并覆盖不同 key 与冲突 key 的测试。



##########
protocol/jsonrpc/server.go:
##########
@@ -194,6 +188,16 @@ func (s *Server) handlePkg(conn net.Conn) {
        }
 }
 
+func contextFromRequest(r *http.Request) context.Context {
+       ctx := r.Context()

Review Comment:
   [P1] 生产 JSON-RPC 请求没有可取消的 context
   
   生产路径中的 `r` 来自手工 `http.ReadRequest(bufReader)`,不是 
`net/http.Server`;按相同构造执行的探针确认 `contextFromRequest(r).Done()` 为 nil。因此这里改用 
`r.Context()` 仍不会在客户端断连时取消阻塞中的 RPC,而新增测试使用 
`httptest.NewRequestWithContext`,绕过了这个生产前提。请为连接建立 cancelable context 
并在连接/读循环结束时 cancel,或由 `net/http.Server` 管理生命周期,再用断连加阻塞 invoker 的测试验证。



##########
metadata/client.go:
##########
@@ -55,6 +55,15 @@ func GetMetadataFromMetadataReport(revision string, instance 
registry.ServiceIns
 }
 
 func GetMetadataFromRpc(revision string, instance registry.ServiceInstance) 
(*info.MetadataInfo, error) {
+       return GetMetadataFromRpcWithContext(context.Background(), revision, 
instance)

Review Comment:
   [P1] 实际 metadata 调用方仍全部走 Background 入口
   
   当前仓库中非测试调用只有 `service_instances_changed_listener_impl.go` 的两处 
`GetMetadataFromRpc(...)`,它们都会进入这里并固定使用 `context.Background()`;新增的 
`GetMetadataFromRpcWithContext` 只在测试中被直接调用。因此生产 metadata v1/v2 
路径仍拿不到调用方取消、deadline 或 value,新增 API 没有闭合 Issue #3557 的真实入口。请把 context 沿 
listener/GetMetadataInfo 调用链传到这里并改用 WithContext 入口,再从该生产入口增加取消/期限回归测试。



##########
metadata/client.go:
##########
@@ -160,15 +170,17 @@ type remoteMetadataServiceV1 struct {
 }
 
 // getMetadataInfo fetches metadata via RPC using the dubbo:// protocol 
(Hessian2 serialization).
-// TODO(context-propagation): ctx is not yet forwarded to the invoker; 
cancellation is not respected.
-func (m *remoteMetadataServiceV1) getMetadataInfo(_ context.Context, revision 
string) (*info.MetadataInfo, error) {
+func (m *remoteMetadataServiceV1) getMetadataInfo(ctx context.Context, 
revision string) (*info.MetadataInfo, error) {
        const methodName = "getMetadataInfo"
        // Use interface{} as reply parameter to accept any type (MetadataInfo 
or string)
        // This avoids panic when Java returns String instead of MetadataInfo
        var rawResult any
        inv, _ := generateInvocation(m.invoker.GetURL(), methodName, revision, 
&rawResult, constant.CallUnary)
+       if rpcInv, ok := inv.(*invocation.RPCInvocation); ok {
+               rpcInv.SetContext(ctx)
+       }
 
-       res := m.invoker.Invoke(context.Background(), inv)
+       res := m.invoker.Invoke(ctx, inv)

Review Comment:
   [P1] metadata v1 仍不响应 context 取消
   
   即使直接调用 WithContext,这里的 v1 实际链路仍是 `DubboInvoker.Invoke -> 
ExchangeClient.Request -> Getty Client.Request`:context 只用于 tracing,底层等待只监听固定 
timeout 和 `PendingResponse.Done`,没有监听 `ctx.Done()`。隔离探针让 transport 阻塞后取消 
context,当前 Head 在 100 ms 内仍未返回,直到手工释放 transport。因此请求、连接和 pending-response 
状态仍会占用到固定超时。请让 Dubbo remoting request API 接收 context,并在取消时移除 pending 
response,同时增加阻塞 transport 的 cancellation/deadline 测试。



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