kou opened a new pull request, #54685: URL: https://github.com/apache/arrow/pull/54685
### Rationale for this change `arrow-flight-sql-odbc-test` sometimes aborts with `GRPC_CALL_ERROR_TOO_MANY_OPERATIONS` in `ClientReaderWriter::Finish()` of the handshake stream. This happens when a server finishes a handshake before the client writes or reads. For example, `NoOpAuthHandler` on the server side and Dremio do this. In this case: 1. `GrpcClientAuthSender::Write()` (or `GrpcClientAuthReader::Read()`) fails and calls `Finish()` on the handshake stream. 2. The returned status is OK, so the client auth handler and `AuthenticateInternal()` continue. 3. `AuthenticateInternal()` calls `WritesDone()`, drains the stream and calls `Finish()` again. 4. gRPC rejects the second `RECV_STATUS_ON_CLIENT` with `GRPC_CALL_ERROR_TOO_MANY_OPERATIONS` and aborts the process. `GRPC_TRACE=api` shows this sequence: ```text SEND_MESSAGE -> ERROR: Attempt to send message after stream was closed. RECV_INITIAL_METADATA + RECV_STATUS_ON_CLIENT -> OK (Finish() by GrpcClientAuthSender::Write()) SEND_CLOSE_FROM_CLIENT (WritesDone()) RECV_MESSAGE (drain) RECV_STATUS_ON_CLIENT -> GRPC_CALL_ERROR_TOO_MANY_OPERATIONS (Finish() by AuthenticateInternal()) ``` Whether the server finishes before the client writes is a race. So it's flaky. ### What changes are included in this PR? * Wrap the handshake stream so that `Finish()` is called at most once. The second and later calls return the status of the first call. * Skip `WritesDone()` and draining in `AuthenticateInternal()` when the stream is already finished. ### Are these changes tested? Yes. * Added `TestNoOpAuthHandler.ReadWriteAfterFinish`. It reproduces the double `Finish()` deterministically: it aborts without this change and passes with this change. * With the same configuration as the `ubuntu-cpp-odbc` CI job (bundled static dependencies, release build) and without Dremio, `arrow-flight-sql-odbc-test` aborted 28 times in 300 runs without this change and 0 times in 300 runs with this change. * `arrow-flight-test` and `arrow-flight-sql-test` pass. ### Are there any user-facing changes? Yes. Flight clients don't abort the process when a server finishes a handshake early. **This PR contains a "Critical Fix".** This fixes a bug that causes a crash: `FlightClient::Authenticate()` may abort the process when a server finishes a handshake before the client writes or reads. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [ ] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #49818 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
