[C++][Flight] Flight benchmark doesn't work anymore

[pitrou]

Describe the bug, including details regarding any error messages, version, and platform.

On my local build:

$ /build/build-release/relwithdebinfo/arrow-flight-benchmark 
Testing method: DoGet
Using spawned TCP server
Server running with pid 71195
Server host: localhost
Server port: 31337
Failed with error: << IOError: Flight returned unavailable error, with message: failed to connect to all addresses; last error: UNKNOWN: ipv4:127.0.0.1:31337: Failed to connect to remote host: Connection refused. Detail: Unavailable

Component(s)

Benchmarking, C++, FlightRPC

[pitrou]

@lidavidm @kou

[kou]

Hmm. It seems that

arrow/cpp/src/arrow/flight/flight_benchmark.cc

Line 134 in 420c01a

if (client->DoAction(call_options, action).ok()) {

returns OK even when arrow-flight-perf-server isn't listen a socket.

[lidavidm]

That isn't surprising for gRPC; streams only give errors when finished. (Another reason not to wrap gRPC so heavily.)

[lidavidm]

A while ago DoAction would eagerly consume all results up front (the wrong behavior!) meaning that it would give errors immediately. This was fixed by a contributor but had the side effect of breaking this kind of usage (which was wrong all along, but hidden by Flight being a heavy wrapper over another library that we don't properly understand).

[kou]

Ah, I see. The following works. Is it OK? Or do we have more simpler way?

diff --git a/cpp/src/arrow/flight/flight_benchmark.cc b/cpp/src/arrow/flight/flight_benchmark.cc
index f53b1c6dce..057ef15c3c 100644
--- a/cpp/src/arrow/flight/flight_benchmark.cc
+++ b/cpp/src/arrow/flight/flight_benchmark.cc
@@ -131,7 +131,8 @@ struct PerformanceStats {
 Status WaitForReady(FlightClient* client, const FlightCallOptions& call_options) {
   Action action{"ping", nullptr};
   for (int attempt = 0; attempt < 10; attempt++) {
-    if (client->DoAction(call_options, action).ok()) {
+    auto result_stream_result = client->DoAction(call_options, action);
+    if (result_stream_result.ok() && (*result_stream_result)->Drain().ok()) {
       return Status::OK();
     }
     std::this_thread::sleep_for(std::chrono::milliseconds(1000));

[lidavidm]

That should be good.

[kou]

#41793

[kou]

Issue resolved by pull request 41793
#41793