lizining1231 opened a new issue, #3748: URL: https://github.com/apache/dubbo-go/issues/3748
### Background PR #3746 added an optional 32 KiB write buffer to the Triple streaming client. When small messages are sent in succession, the buffer can combine writes to the underlying `io.Pipe`. This issue proposes enabling it by default while retaining an explicit way to disable it. With write buffering enabled by default, `WithWriteBuffering()` would only restate the default. `WithUnaryFastPath()` likewise became redundant when the unary fast path was enabled by default, so it can be removed at the same time. ### Two buffering defects exposed by the new default Write buffering previously had to be enabled manually, so most tests did not exercise `streamBufferWriter`. Enabling it by default exposes two issues that directly affect existing streaming calls: zero-length writes and context cancellation. #### 1. Zero-length writes are swallowed `Send(nil)` eventually makes a zero-length `Write` call. Without buffering, `duplexHTTPCall.Write` starts the HTTP request even when there are no bytes to write. With buffering, writing zero bytes leaves the buffer empty, and `flushLocked` returns immediately because `buf.Len() == 0`. The request is therefore never sent. `BlockUntilResponseReady` then waits indefinitely for `responseReady`; it does not observe context cancellation, so even the deadline cannot end the wait. As a result, a stream that sends only a nil message hangs and causes the package test run to time out. Fix: Pass zero-length writes directly to the underlying writer without buffering them. Leave any existing nonempty buffered data in place, and return errors from the underlying write to the caller. #### 2. Small messages bypass the context check `duplexHTTPCall.Write` checks the context before writing. If the context has been canceled, it returns `CodeCanceled` and records the error so that subsequent reads also observe the failure. With buffering enabled, however, a small message is written only to the in-memory buffer and bypasses this check, so `Send` may incorrectly report success. After the zero-length write fix, a nil message goes directly to the underlying writer; reproducing this separate issue therefore requires a nonempty small message that is buffered. Fix: Have `streamBufferWriter` call `preWriteChecker` before accepting data, with `duplexHTTPCall` implementing the check. When the context is valid, the check should only inspect its state: the request should still start, and its headers should still be sent, on the first flush. When the context has been canceled, follow the direct-write path: start the request, record the error, and return `CodeCanceled`. Subsequent reads should also return the cancellation error. #### Reproduction ```bash # A: A stream that sends only a nil message waits until the test times out. go test -run '^TestStreamForServer$/^nil-message$' -count=1 -timeout=5s ./protocol/triple/triple_protocol # B: Send incorrectly returns nil after context cancellation instead of CodeCanceled. go test -run '^TestServer$/^http2$/^grpc$/^proto$/^cumsum_cancel_before_send$' -count=1 -timeout=15s ./protocol/triple/triple_protocol ``` ### Proposed changes 1. Enable streaming write buffering by default in the client and framework configurations. 2. Remove the two redundant enable options while retaining their corresponding explicit disable options. 3. Fix the zero-length write and context cancellation issues exposed by the new default. 4. Update the relevant tests and A/B benchmarks to reflect the new default behavior. ### Behaviors requiring test coverage - Write buffering is enabled by default, and the explicit disable option still selects the previous call path. - A zero-length write starts the request; existing buffered data retains its order; closing the request side flushes any remaining data. - Small messages honor the send error contract both before and after context cancellation, and subsequent reads also receive the cancellation error. - Streaming calls for both protocols, as well as unary calls that use the duplex path, use write buffering as expected. -- 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]
