github-actions[bot] commented on code in PR #69991:
URL: https://github.com/apache/doris/pull/69991#discussion_r4447064294
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/source/HiveScanNode.java:
##########
@@ -256,22 +261,20 @@ public void startSplit(int numBackends) {
try {
List<Split> allFiles = Lists.newArrayList();
getFileSplitByPartitions(
- cache,
Collections.singletonList(partition), allFiles, bindBrokerName,
Review Comment:
[P1] Preserve one table-wide TABLESAMPLE target across partition batches.
`startSplit()` now calls `getFileSplitByPartitions()` once per
`Lists.partition` batch, but that method runs `selectFiles(fileCaches)` with
the full `TABLESAMPLE(N ROWS)` target each time. A 32-partition table with the
default batch size 16 therefore selects roughly 2N rows/files, whereas the
non-batch path computes one target over all partitions; percent sampling also
changes through per-batch rounding. Carry the remaining sample budget or
materialize the sample once before batching, and add a multi-batch ROWS
regression test.
##########
be/src/exec/scan/split_source_connector.cpp:
##########
@@ -191,4 +203,38 @@ Status RemoteSplitSourceConnector::get_next(bool*
has_next, TFileRangeDesc* rang
return Status::OK();
}
+Status RemoteSplitSourceConnector::_fetch_batch(TFetchSplitBatchResult* result,
+ const TFetchSplitBatchRequest&
request) {
+ Status status;
+ FrontendServiceConnection
coord(_state->exec_env()->frontend_client_cache(),
+ _state->get_query_ctx()->coord_addr,
&status);
+ RETURN_IF_ERROR(status);
+ try {
+ coord->fetchSplitBatch(*result, request);
+ } catch (const std::exception& e) {
+ return Status::IOError<false>("Failed to get batch of split source:
{}", e.what());
+ }
+ return Status::OK();
+}
+
+Status RemoteSplitSourceConnector::close() {
+ stop();
+ // Older FEs do not advertise support for the source-close request.
Review Comment:
[P1] Keep source-close cleanup retryable until FE acknowledges it.
`_close_requested` is set before `_fetch_batch()`, so a transient transport/FE
failure leaves the source registered when the request did not reach FE, while
all later close calls return immediately. On a large scan this can refill that
backend's bounded queue and stall split delivery to other backends after a
local LIMIT; the non-OK close status also cancels the query. Set the latch only
after success or retain a bounded retry/best-effort cleanup path, and avoid
turning a transient cleanup failure into the scan's close status.
--
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]
