Doris-Breakwater commented on issue #70279:
URL: https://github.com/apache/doris/issues/70279#issuecomment-6654196968

   Breakwater-GitHub-Analysis-Slot: slot_fb0669e965d4
   
   **Triage: the error-classification inconsistency is supported by the source 
at `72e91896bbdd8760f8bd4e488e5672406b40ce52` (high confidence).** This is an 
FE incremental-read/transaction-fence issue with a connector retry consequence. 
The issue is currently open with no labels. I verified the pinned revision and 
the cited connector revision; I did not reproduce this on a cluster, run tests, 
or verify the claim that today's master is unchanged. No implementation changes 
were made.
   
   **Verified mechanism**
   
   - Strongly consistent shared-nothing reads take the transaction-watermark 
drain and publisher-lock synchronization path. Both deadline expiration in 
`waitForPreviousTransactions()` and failure to obtain the publisher read lock 
in time throw a plain `UserException`. That exception defaults to 
`ERR_UNKNOWN_ERROR`, rather than 5101. Bounded shared-storage reads instead use 
`TSOService.waitForReadableWindow()`, which creates an 
`IncrWindowNotReadyException` with 5101 for a visibility deadline and 5100 for 
a future window or master change. See the [fence 
implementation](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/qe/TimeBasedChangeVisibleWaiter.java#L176-L212),
 [classic timeout 
sites](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/qe/TimeBasedChangeVisibleWaiter.java#L274-L337),
 and [shared-storage classification](https://github.com/apache
 
/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/tso/TSOService.java#L352-L402).
   - The missing classification persists across component boundaries. Planning 
wraps the exception; `StmtExecutor` specifically preserves the code of 
`IncrWindowNotReadyException`, while other planning errors take the 
generic-error path. The master fence RPC likewise serializes structured 
window-error fields only for that exception type. Flight emits `UNAVAILABLE` 
with `doris-error-code` metadata only when query state holds 5100/5101; 
otherwise it emits `INTERNAL`. Consequently, changing only the message, or 
assigning a code to an otherwise plain `UserException`, would not complete the 
fix through these paths. See [planner-error 
handling](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L841-L860),
 [master RPC 
handling](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java#L3466-L3503),
 and [Flig
 ht 
metadata](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java#L335-L345).
   - The cited connector retries the same statement only when ADBC details 
contain `doris-error-code=5101`. Its `source.changelog.visible-wait-timeout` 
controls the client retry budget; it cannot make an unclassified FE timeout 
eligible for retry. **5100 is not retried by this connector revision**, so 
returning 5100 for an actual visibility timeout would not address the reported 
failure. See [connector retry 
logic](https://github.com/apache/doris-flink-connector/blob/9550f9a0ebb16d95964ec65340f877b120f15403/flink-doris-connector/flink-doris-connector-base/src/main/java/org/apache/doris/flink/source/reader/DorisFlightValueReader.java#L143-L172).
   
   **Reproduction qualification**
   
   The classic conflict check considers non-final transactions touching the 
requested tables with transaction ID at or below the watermark; it does not 
compare their commit TSO with the requested window. This supports the proposed 
long-load blocker, provided its transaction is registered before the fence 
captures the watermark. It does not by itself establish that this conservative 
drain is incorrect. See [transaction 
predicate](https://github.com/apache/doris/blob/72e91896bbdd8760f8bd4e488e5672406b40ce52/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java#L2715-L2733).
   
   The identical slow-load scenario need not time out in shared-storage mode: 
its bounded wait filters registered commit TSOs by the window end and target 
tables. A transaction outside that range may correctly leave the read 
unblocked. To demonstrate shared-storage 5101, hold an in-range registered 
transaction unfinished. Also choose the reproduction's end timestamp at or 
below the master's current TSO physical time; otherwise classic validation can 
reject the query before reaching the visibility wait.
   
   **Recommended next steps**
   
   1. Classify the actual classic visibility deadlines as 5101 using the 
existing typed window-error propagation, or an equivalent mechanism verified 
through planning, RPC, and Flight. Preserve the drain, publisher 
synchronization, and useful transaction/table diagnostics. Keep interruption, 
transaction-check failure, and transport failure distinguishable from a 
visibility deadline.
   2. Include follower execution in scope: classic follower journal replay has 
additional generic timeout exits. The fence RPC currently grants extra response 
time only to bounded cloud reads, so a classic follower can encounter a socket 
timeout before receiving a typed master timeout. Cover these boundaries 
explicitly rather than considering the two master-side throws sufficient.
   3. Add regression coverage for classic drain timeout and publisher-lock 
timeout, direct and follower execution, query-state code preservation, and 
Flight metadata. Verify the connector retries an unchanged window within its 
budget, succeeds after the blocker clears, and stops when the budget expires. 
Existing typed-error tests exercise cloud paths and do not establish classic 
coverage.
   
   **Still needed for runtime confirmation:** an executable table DDL/load 
script and exact timestamped SQL; confirmation that the blocking transaction 
was registered before fence acquisition; actual `change_visible_timeout_ms` and 
consistency settings; connected FE role; FE query ID/logs with transaction 
states; connector build/options and full ADBC error details showing metadata. 
These are needed to validate the reproduction and observed failover, but the 
source is already sufficient to justify an error-propagation fix. There is no 
evidence here establishing data loss or the frequency of this failure in 
deployed workloads.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to