lizining1231 commented on PR #3706: URL: https://github.com/apache/dubbo-go/pull/3706#issuecomment-5437094045
> 1. the ci fails > 2. > add an opt-in UnaryFastPath toggle, off by default > > why not just set it to on by default, is there a competiable issue between fastpath call and duplex call thanks for review **About compatibility issue** There is no compatibility break between the fast path and the duplex call, they are wire- and API-compatible, but they have deliberately different runtime behavior, and we didn't want to silently switch every existing user onto a new execution path before it's validated at scale. **1. Wire-compatible:** TestUnaryFastPathWireConsistent asserts both paths emit byte-identical requests. **2. API-compatible:** WithUnaryFastPath is a new opt-in ClientOption. With it off (the default), callers keep using duplexHTTPCall unchanged, no public API signature or behavior change. **3. Scoped to unary-on-Triple only:** the switch applies only when spec.StreamType == StreamTypeUnary && UnaryFastPath. Client/server/bidi streaming always use duplexHTTPCall, and the gRPC protocol never reads this toggle, so those paths are untouched. **4. Contract preserved:** the StreamingClientConn concurrency contract (Send / RequestHeader / CloseRequest may race) is satisfied, writeMu serializes Write vs CloseWrite, and Write after CloseWrite returns io.EOF, mirroring the duplex pipe-close semantics. Covered by unary_fastpath_concurrency_test.go under -race. **About ci fail** CI is red on the integrate test. Likely the known flaky samples step: the last go-client call hits the Java server, which only accepts application/grpc+proto while the dubbo-go client sends application/proto, so that step 415s whenever the Java server starts in time — it only "passes" when the Java server fails to start and the step is skipped. The root cause is in the samples test flow, not this PR. Fix in progress. **About the triple-unary optimization option defaulting to off** When I implemented it, I considered that users might not want to apply this change, so I set it to off by default. I have now changed it to on by default. -- 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]
