This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 1f41d4223bf [fix](arrow-flight) Let GetFlightInfo pass the session
layer's Flight status through again (#68221)
1f41d4223bf is described below
commit 1f41d4223bf30753f524b92c8e273b6211415d36
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Sun Sep 20 11:20:52 2026 +0800
[fix](arrow-flight) Let GetFlightInfo pass the session layer's Flight
status through again (#68221)
### What problem does this PR solve?
Issue Number: None
Related PR: #67883, #67820, #67966, #68101
Problem Summary:
`DorisFlightSqlProducer.getFlightInfoStatement` used to rethrow a
`FlightRuntimeException` as is (#67883), so the status the session layer
chose reaches the client: `UNAVAILABLE` from the session's command lock
and `UNAUTHENTICATED` from a closed session (#67900, #67966), and
whatever a refused session is answered with. #67820 rewrote that catch
block into a catch-all that lets only its two incremental-window errors
through (by `doris-error-code` metadata) and wraps every other
`FlightRuntimeException` as `INTERNAL: get flight info statement failed,
<message>` -- the wrapping #67883 had removed. The producer's other
entry points (`setSessionOptions`, `streamMetadata`) still let the
status through and say they do it "as in getFlightInfoStatement".
This PR restores the passthrough ahead of the catch-all. The window
errors are `FlightRuntimeException`s built by `queryFailure`, so
#67820's metadata special case is subsumed and removed; a non-Flight
failure is still wrapped as `INTERNAL` with the same message.
Found through #68101: its `test_connection_quota` asserts the
`RESOURCE_EXHAUSTED` the connection pool answers a refused Flight
session with, and since the pipelines compile a PR merged into master,
every run after #67820 landed got the wrapped `INTERNAL` instead.
---
.../doris/arrowflight/DorisFlightSqlProducer.java | 19 ++++++--------
.../arrowflight/DorisFlightSqlProducerTest.java | 29 +++++++++++-----------
2 files changed, 21 insertions(+), 27 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java
b/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java
index 6bc9cada585..656da1ba4fb 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java
@@ -24,7 +24,6 @@ import
org.apache.doris.arrowflight.protocol.FlightProtocolAdapter;
import org.apache.doris.arrowflight.results.FlightSqlEndpointsLocation;
import org.apache.doris.arrowflight.results.FlightSqlResultCacheEntry;
import org.apache.doris.arrowflight.sessions.FlightSessionsManager;
-import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.IncrWindowNotReadyException;
import org.apache.doris.common.Status;
import org.apache.doris.common.util.DebugUtil;
@@ -351,18 +350,14 @@ public class DorisFlightSqlProducer implements
FlightSqlProducer, AutoCloseable
return
FlightProtocolAdapter.of(connectContext).callCommand(connectContext,
() -> executeQueryStatement(context.peerIdentity(),
connectContext, request.getQuery(),
descriptor));
+ } catch (FlightRuntimeException e) {
+ // Already carries the status meant for the client - UNAVAILABLE
from the session's
+ // command lock or from an incremental window that is not ready
(queryFailure, with its
+ // doris-error-code metadata), UNAUTHENTICATED from a closed
session, and whatever the
+ // session layer refuses a session with. Wrapping it as INTERNAL
would hide that, as it
+ // did between #67820 and this fix; the other entry points below
let it through too.
+ throw e;
} catch (Throwable e) {
- if (e instanceof FlightRuntimeException) {
- FlightRuntimeException flightError = (FlightRuntimeException)
e;
- ErrorFlightMetadata metadata = flightError.status().metadata();
- if (metadata.containsKey("doris-error-code")) {
- String code = metadata.get("doris-error-code");
- if
(Integer.toString(ErrorCode.ERR_INCR_WINDOW_NOT_READY.getCode()).equals(code)
- ||
Integer.toString(ErrorCode.ERR_INCR_VISIBLE_WAIT_TIMEOUT.getCode()).equals(code))
{
- throw flightError;
- }
- }
- }
String errMsg = "get flight info statement failed, " +
e.getMessage();
LOG.error(errMsg, e);
throw
CallStatus.INTERNAL.withDescription(errMsg).withCause(e).toRuntimeException();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/arrowflight/DorisFlightSqlProducerTest.java
b/fe/fe-core/src/test/java/org/apache/doris/arrowflight/DorisFlightSqlProducerTest.java
index ae1d07240a0..6fb16d62fbf 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/arrowflight/DorisFlightSqlProducerTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/arrowflight/DorisFlightSqlProducerTest.java
@@ -107,29 +107,37 @@ public class DorisFlightSqlProducerTest {
}
}
+ // A Flight status chosen by the session layer reaches the client as is,
whatever it is: the
+ // session's command lock (UNAVAILABLE), a closed session
(UNAUTHENTICATED), a refused session
+ // (RESOURCE_EXHAUSTED). Only a non-Flight failure is wrapped as INTERNAL.
@Test
- public void testGetFlightInfoWrapsOtherFlightErrors() throws Exception {
+ public void testGetFlightInfoPassesOtherFlightErrorsThrough() throws
Exception {
for (CallStatus status : new CallStatus[] {CallStatus.INTERNAL,
CallStatus.UNAVAILABLE,
- CallStatus.INVALID_ARGUMENT, CallStatus.UNAUTHENTICATED}) {
+ CallStatus.INVALID_ARGUMENT, CallStatus.UNAUTHENTICATED,
CallStatus.RESOURCE_EXHAUSTED}) {
FlightRuntimeException failure = status.withDescription("other
flight failure").toRuntimeException();
- assertLegacyFlightWrapper(failure, getFlightInfoFailure(failure));
+ Assertions.assertSame(failure, getFlightInfoFailure(failure));
}
}
@Test
- public void testGetFlightInfoWrapsOtherBusinessErrors() throws Exception {
+ public void testGetFlightInfoPassesOtherBusinessErrorsThrough() throws
Exception {
ErrorFlightMetadata metadata = new ErrorFlightMetadata();
metadata.insert("doris-error-code",
Integer.toString(ErrorCode.ERR_UNKNOWN_ERROR.getCode()));
FlightRuntimeException failure =
CallStatus.UNAVAILABLE.withDescription("other business failure")
.withMetadata(metadata).toRuntimeException();
- assertLegacyFlightWrapper(failure, getFlightInfoFailure(failure));
+ Assertions.assertSame(failure, getFlightInfoFailure(failure));
}
@Test
public void testGetFlightInfoWrapsNonFlightErrors() throws Exception {
RuntimeException failure = new RuntimeException("session lookup
failed");
- assertLegacyFlightWrapper(failure, getFlightInfoFailure(failure));
+ FlightRuntimeException result = getFlightInfoFailure(failure);
+ Assertions.assertEquals(FlightStatusCode.INTERNAL,
result.status().code());
+ Assertions.assertSame(failure, result.getCause());
+ Assertions.assertEquals("get flight info statement failed, " +
failure.getMessage(),
+ result.status().description());
+
Assertions.assertFalse(result.status().metadata().containsKey("doris-error-code"));
}
private FlightRuntimeException getFlightInfoFailure(RuntimeException
failure) throws Exception {
@@ -145,15 +153,6 @@ public class DorisFlightSqlProducerTest {
}
}
- private void assertLegacyFlightWrapper(RuntimeException failure,
FlightRuntimeException result) {
- Assertions.assertEquals(FlightStatusCode.INTERNAL,
result.status().code());
- Assertions.assertNotSame(failure, result);
- Assertions.assertSame(failure, result.getCause());
- Assertions.assertEquals("get flight info statement failed, " +
failure.getMessage(),
- result.status().description());
-
Assertions.assertFalse(result.status().metadata().containsKey("doris-error-code"));
- }
-
@BeforeEach
public void setUp() {
// ConnectContext.init() only reaches Env when this is false; keep it
true so the
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
