[ 
https://issues.apache.org/jira/browse/ARROW-16592?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17538443#comment-17538443
 ] 

David Li commented on ARROW-16592:
----------------------------------

Aha. Thank you so much!

That reproduces it for me. So what happens is that when writing data, gRPC 
crucially does not tell you if there was an error or not. It just returns 
boolean success/failure. (This is because there may be queueing behind the 
scenes, as far as I understand.) That's also why you can write data "after" the 
failure. 

Before we just swallowed the error and continued on. Then you would only get 
the error once you finished "writing" (dropping all the data on the floor). Now 
we don't swallow the error - that's probably inadvertent, since we refactored 
how things work here. But the error here is "meaningless" because gRPC only 
gives a boolean. You can observe it if you do this:

{code:python}
try:
    # starts failing at this exact number; before that the error is still 
cancelled
    try:
        for i in range(27061):
            # print(f"write {i}")
            writer.write_batch(pa.record_batch([[1]], schema=schema))
    except flight.FlightError as e:
        traceback.print_exc()
        print(e.extra_info)

    writer.close()
except flight.FlightError as e:
    traceback.print_exc()
    print(e.extra_info)
{code}
The 'real' exception is still there if you call close().

So how does Close() get the "real" error? gRPC will give the _reader_ side of 
the bidirectional stream the error. But we can't actually do this inside the 
writer, because gRPC will crash if two threads try to read the stream at once. 
Now, we do have a lock. So I think the "right" thing to do will be, if we get 
an error while writing, we need to lock the read side of the stream, drain it, 
and raise the real error, instead of propagating the "fake" error (or 
swallowing it, which would mean your client could do a lot of unnecessary work 
before getting the error).

> [FlightRPC][Python] Regression in DoPut error handling
> ------------------------------------------------------
>
>                 Key: ARROW-16592
>                 URL: https://issues.apache.org/jira/browse/ARROW-16592
>             Project: Apache Arrow
>          Issue Type: Bug
>            Reporter: Lubo Slivka
>            Assignee: David Li
>            Priority: Major
>
> In PyArrow 8.0.0, any error raised while handling DoPut on the server results 
> in FlightInternalError on the client.
> In PyArrow 7.0.0, errors raised while handling DoPut are propagated/converted 
> to non-internal errors.
> —
> Example: on 7.0.0, raising FlightCancelledError while handling DoPut on the 
> server would propagate that error including extra_info all the way to the 
> FlightClient. This is not the case anymore on 8.0.0.
> The FlightInternalError contains extra detail that is derived from the 
> cancelled error though:
> {code:java}
> /arrow/cpp/src/arrow/flight/client.cc:363: Close() failed: IOError: <error 
> message from FlightError is here>. Detail: Cancelled. gRPC client debug 
> context: {"created":"@1652777650.446052211","description":"Error received 
> from peer 
> ipv4:127.0.0.1:16001","file":"/opt/vcpkg/buildtrees/grpc/src/85a295989c-6cf7bf442d.clean/src/core/lib/surface/call.cc","file_line":903,"grpc_message":"<error
>  message from FlightError is here>. Detail: Cancelled","grpc_status":1}. 
> Client context: OK. Detail: Cancelled
>  {code}
> Note: skimming through the code, it seems this problem is not unique to 
> PyArrow.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to