github-actions[bot] commented on code in PR #66802:
URL: https://github.com/apache/doris/pull/66802#discussion_r3791684742
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -448,16 +470,50 @@ Status FileScannerV2::_open_impl(RuntimeState* state) {
DORIS_CHECK(_table_reader != nullptr);
RETURN_IF_ERROR(_init_expr_ctxes());
RETURN_IF_ERROR(_init_table_reader(_current_range));
+ // Refine the first source split before yielding the scanner worker.
Other scanners may be
+ // waiting for its row-group children, so deferring publication until
a later get_block()
+ // turn could let those waiters occupy the scan thread pool ahead of
the producer.
+ bool eos = false;
+ RETURN_IF_ERROR(_prepare_next_split(&eos));
Review Comment:
[P1] Refresh the split digest after late Runtime Filters
This eagerly prepares the first split inside `open()`, before
`ScannerScheduler` calls `try_append_late_arrival_runtime_filter()`. If an RF
arrives during that work, a generated Parquet child or first ORC split keeps
`digest(P)`, but the reader-null refresh changes its conjuncts to `P AND RF`
without updating the digest/coverage flag. A Condition Cache MISS that reaches
EOF can then publish `P AND RF` survivors under `P`; a later `P`-only HIT will
skip granules containing valid rows. Defer preparation until after the RF hook,
or recompute/invalidate the split cache identity on reader-null refresh, and
cover this arrival race with a cache-reuse regression.
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -585,14 +642,51 @@ Status FileScannerV2::_prepare_next_split(bool* eos) {
// advance exactly one scan range and preserve later files in the
same scan.
RETURN_IF_ERROR(_table_reader->abort_split());
COUNTER_UPDATE(_empty_file_counter, 1);
- _state->update_num_finished_scan_range(1);
+ RETURN_IF_ERROR(_complete_current_split());
continue;
}
RETURN_IF_ERROR(status);
if (_table_reader->current_split_pruned()) {
- _state->update_num_finished_scan_range(1);
+ RETURN_IF_ERROR(_complete_current_split());
continue;
}
+ if (_current_split.is_source_split &&
can_refine_source_split(_current_range)) {
+ std::vector<FileScanSplit> generated_splits;
+ bool was_split = false;
+ const auto split_status = _table_reader->build_physical_splits(
+ _current_split, &generated_splits, &was_split);
+ const auto ignored_split_status = _classify_ignored_split_status(
+ split_status,
config::ignore_not_found_file_in_external_table,
+ _should_stop || _io_ctx->should_stop);
+ if (ignored_split_status == IgnoredSplitStatus::NOT_FOUND) {
+ RETURN_IF_ERROR(_table_reader->abort_split());
+ COUNTER_UPDATE(_not_found_file_counter, 1);
+ RETURN_IF_ERROR(_complete_current_split());
+ continue;
+ }
+ if (ignored_split_status == IgnoredSplitStatus::EMPTY) {
+ RETURN_IF_ERROR(_table_reader->abort_split());
+ COUNTER_UPDATE(_empty_file_counter, 1);
+ RETURN_IF_ERROR(_complete_current_split());
+ continue;
+ }
+ RETURN_IF_ERROR(split_status);
+ if (was_split) {
+ RETURN_IF_ERROR(_table_reader->abort_split());
+ const bool has_children = !generated_splits.empty();
+
RETURN_IF_ERROR(_retire_current_source_split(std::move(generated_splits)));
+ if (!has_children) {
+ RETURN_IF_ERROR(_complete_current_split());
+ }
+ continue;
Review Comment:
[P2] Keep FileNumber tied to source ranges
This `continue` skips `FileNumber` for the parent, while every generated Row
Group child later falls through to the existing increment. One FE range
therefore reports R files for R nonempty Row Groups (and zero for an all-empty
refined source), whereas the unsplit/one-group path reports one. That makes the
established file/range metric depend on a BE-local scheduling choice and
renders profiles incomparable. Increment `FileNumber` once for the accepted
source range, and expose generated children through a separately named counter
if needed.
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -1249,6 +1254,40 @@ Status TableReader::prepare_split(const
SplitReadOptions& options) {
return _parse_delete_predicates(options);
}
+Status TableReader::build_physical_splits(const FileScanSplit& source_split,
+ std::vector<FileScanSplit>* splits,
bool* was_split) {
+ DORIS_CHECK(splits != nullptr);
+ DORIS_CHECK(was_split != nullptr);
+ splits->clear();
+ *was_split = false;
+ if (_format != FileFormat::PARQUET || _current_split_pruned ||
+ _current_split_uses_metadata_count || _current_task == nullptr) {
+ return Status::OK();
+ }
+
+ std::unique_ptr<FileReader> reader;
+ RETURN_IF_ERROR(create_file_reader(&reader));
+ DORIS_CHECK(reader != nullptr);
+ RETURN_IF_ERROR(reader->init(_runtime_state));
Review Comment:
[P2] Keep split planning inside the lifecycle profile hierarchy
`prepare_split()`'s `TableReader` timer has ended when this method runs, and
the method opens neither the parent total nor `CreateReaderTime`; its init and
the 0/many-child or build-error close paths also bypass the common `FileReader`
lifecycle timers. Nested `OpenReaderTime`/Parquet `ParseFooterTime` can
therefore contain the primary planning cost while their parents omit it,
materially underreporting scans that use this feature. Use the existing
TableReader/create/init/close scopes here, and cover 0/1/many plus error paths.
--
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]
