github-actions[bot] commented on code in PR #69683:
URL: https://github.com/apache/doris/pull/69683#discussion_r4396311548
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -1119,6 +1119,10 @@ public TFetchSplitBatchResult
fetchSplitBatch(TFetchSplitBatchRequest request) t
result.setSplits(locations);
result.status = new TStatus(TStatusCode.OK);
return result;
+ } catch (SplitSource.SplitPendingException e) {
+ // The backend will retry; an empty OK batch would incorrectly
mean EOF.
+ result.status = new TStatus(TStatusCode.INCOMPLETE);
Review Comment:
[P2] Gate the pending response on backend support
A pre-change BE treats every non-OK split-fetch status as fatal, so during a
rolling upgrade this unconditional `INCOMPLETE` makes the query fail with
`unknown error`. No request capability or BE-version check protects it. With
the new defaults, two Hive partitions can trigger this: A publishes its range
while B lists past the one-second window. Before this PR, that small scan was
synchronous, or one streaming producer collected both ranges before dispatch.
The existing deadline thread is fixed for upgraded BEs; older callers still
need a compatible path. Please negotiate support or gate asynchronous dispatch
for old BEs, and cover upgraded FE + pre-change BE.
##########
be/src/exec/scan/split_source_connector.cpp:
##########
@@ -48,26 +48,36 @@ Status RemoteSplitSourceConnector::get_next(bool* has_next,
TFileRangeDesc* rang
*has_next = false;
if (_scan_index == _scan_ranges.size() && !_last_batch) {
SCOPED_TIMER(_get_split_timer);
- Status coord_status;
- // No need to set timeout because on FE side, there is a max fetch time
- FrontendServiceConnection
coord(_state->exec_env()->frontend_client_cache(),
- _state->get_query_ctx()->coord_addr,
&coord_status);
- RETURN_IF_ERROR(coord_status);
TFetchSplitBatchRequest request;
request.__set_split_source_id(_split_source_id);
request.__set_max_num_splits(config::remote_split_source_batch_size);
TFetchSplitBatchResult result;
- try {
- coord->fetchSplitBatch(result, request);
- if (result.__isset.status && result.status.status_code !=
TStatusCode::OK) {
- return Status::IOError<false>("Failed to get batch of split
source: {}",
- result.status.error_msgs.empty()
- ? "unknown error"
- :
result.status.error_msgs[0]);
+ do {
+ RETURN_IF_CANCELLED(_state);
Review Comment:
[P2] Let local scanner stop interrupt pending retries
`RETURN_IF_CANCELLED` only observes RuntimeState/query cancellation. A local
LIMIT calls `ScannerContext::stop_scanners`, which sets scanner `_should_stop`
without cancelling the enclosing query; FileScanner and FileScannerV2 check
that flag only after this call returns. With parallel scanners (for example, a
nested LIMIT above the serial-read threshold), A can satisfy the limit while B
waits here for a slow partition and another query branch continues. B now
retries across every wait window while holding `_range_lock`, a scan worker and
the fragment context until listing or the entire query ends. Previously this
call returned after one bounded fetch. Please observe local scan termination
between retries or return control to the scanner, and cover this with
RuntimeState still healthy.
--
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]
