davidhcoe commented on code in PR #4322:
URL: https://github.com/apache/arrow-adbc/pull/4322#discussion_r3271063159
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -48,7 +49,19 @@ type reader struct {
// kicks off a goroutine for each endpoint and returns a reader which
// gathers all of the records as they come in.
-func newRecordReader(ctx context.Context, alloc memory.Allocator, cl
*flightsql.Client, info *flight.FlightInfo, clCache gcache.Cache, bufferSize
int, opts ...grpc.CallOption) (rdr array.RecordReader, err error) {
+//
+// logger may be nil; in that case a no-op logger is used internally.
+// When supplied it receives structured records describing every endpoint
+// stream's lifecycle (start, first batch received, completion, failure).
+// These records are essential when diagnosing transient stream failures
+// such as "[FlightSQL] error reading from server: EOF (Unavailable; DoGet:
+// endpoint N: [])" because they record exactly which endpoint failed, how
+// many batches/rows had already been received, and how long the stream had
+// been open at the time of failure. Without these records the operator
+// otherwise has only the bare gRPC EOF to work with, which carries no
+// progress or location information.
+func newRecordReader(ctx context.Context, alloc memory.Allocator, cl
*flightsql.Client, info *flight.FlightInfo, clCache gcache.Cache, bufferSize
int, logger *slog.Logger, opts ...grpc.CallOption) (rdr array.RecordReader, err
error) {
Review Comment:
Done — extracted a recordReaderConfig struct (alloc, cl, info, clientCache,
bufferSize, logger) and changed the signature to newRecordReader(ctx
context.Context, cfg recordReaderConfig, opts ...grpc.CallOption). Logger is
still optional (nil → no-op via safeLogger). Updated all 13 call sites.
--
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]
