github-actions[bot] commented on code in PR #65218:
URL: https://github.com/apache/doris/pull/65218#discussion_r3526246465
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -770,10 +781,70 @@ void FileScannerV2::update_realtime_counters() {
if (_file_reader_stats == nullptr) {
return;
}
- const int64_t bytes_read = _file_reader_stats->read_bytes;
+ DORIS_CHECK(_file_cache_statistics != nullptr);
+ const int64_t bytes_read =
cast_set<int64_t>(_file_reader_stats->read_bytes);
+ auto* local_state = static_cast<FileScanLocalState*>(_local_state);
+ const auto deltas = _collect_realtime_counter_deltas(
+ *_file_reader_stats, *_file_cache_statistics, &_last_read_bytes,
&_last_read_rows,
+ &_last_bytes_read_from_local, &_last_bytes_read_from_remote);
+
+ COUNTER_UPDATE(local_state->_scan_bytes, deltas.scan_bytes);
+ COUNTER_UPDATE(local_state->_scan_rows, deltas.scan_rows);
+
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_rows(deltas.scan_rows);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes(deltas.scan_bytes);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_local_storage(
+ deltas.scan_bytes_from_local_storage);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_remote_storage(
+ deltas.scan_bytes_from_remote_storage);
+
COUNTER_SET(_file_read_bytes_counter, bytes_read);
COUNTER_SET(_file_read_calls_counter,
cast_set<int64_t>(_file_reader_stats->read_calls));
COUNTER_SET(_file_read_time_counter,
cast_set<int64_t>(_file_reader_stats->read_time_ns));
+
+ DorisMetrics::instance()->query_scan_bytes->increment(deltas.scan_bytes);
+ DorisMetrics::instance()->query_scan_rows->increment(deltas.scan_rows);
+ DorisMetrics::instance()->query_scan_bytes_from_local->increment(
+ deltas.scan_bytes_from_local_storage);
+ DorisMetrics::instance()->query_scan_bytes_from_remote->increment(
+ deltas.scan_bytes_from_remote_storage);
+}
+
+FileScannerV2::RealtimeCounterDeltas
FileScannerV2::_collect_realtime_counter_deltas(
+ const io::FileReaderStats& file_reader_stats,
+ const io::FileCacheStatistics& file_cache_statistics, int64_t*
last_read_bytes,
+ int64_t* last_read_rows, int64_t* last_bytes_read_from_local,
+ int64_t* last_bytes_read_from_remote) {
+ DORIS_CHECK(last_read_bytes != nullptr);
+ DORIS_CHECK(last_read_rows != nullptr);
+ DORIS_CHECK(last_bytes_read_from_local != nullptr);
+ DORIS_CHECK(last_bytes_read_from_remote != nullptr);
+
+ const int64_t read_bytes = cast_set<int64_t>(file_reader_stats.read_bytes);
+ const int64_t read_rows = cast_set<int64_t>(file_reader_stats.read_rows);
Review Comment:
This row counter source is not populated by the real FileScannerV2 path.
`FileScannerV2::init()` wires `_io_ctx->file_reader_stats` into the format_v2
reader stack, but the shared stats object is only updated by
`TracingFileReader`, which records `read_calls`, `read_bytes`, and
`read_time_ns`. `TableReader::get_block()` receives `current_rows` from the
concrete reader and returns them after `finalize_chunk()` without adding them
to `_io_ctx->file_reader_stats->read_rows`; the concrete readers update only
their private `_reader_statistics.read_rows`. As a result `deltas.scan_rows`
stays zero for normal V2 scans, so the new `ScanRows`, IOContext scan rows, and
`query_scan_rows` metric do not actually get published. The tests pass because
they pre-seed `file_reader_stats.read_rows` directly, bypassing this production
path. Please derive the delta from the scanner/TableReader rows that are
actually emitted, or copy those rows into the shared stats before this helper
reads it, and cove
r that path in the test.
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -770,10 +781,70 @@ void FileScannerV2::update_realtime_counters() {
if (_file_reader_stats == nullptr) {
return;
}
- const int64_t bytes_read = _file_reader_stats->read_bytes;
+ DORIS_CHECK(_file_cache_statistics != nullptr);
+ const int64_t bytes_read =
cast_set<int64_t>(_file_reader_stats->read_bytes);
+ auto* local_state = static_cast<FileScanLocalState*>(_local_state);
+ const auto deltas = _collect_realtime_counter_deltas(
+ *_file_reader_stats, *_file_cache_statistics, &_last_read_bytes,
&_last_read_rows,
+ &_last_bytes_read_from_local, &_last_bytes_read_from_remote);
+
+ COUNTER_UPDATE(local_state->_scan_bytes, deltas.scan_bytes);
+ COUNTER_UPDATE(local_state->_scan_rows, deltas.scan_rows);
+
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_rows(deltas.scan_rows);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes(deltas.scan_bytes);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_local_storage(
+ deltas.scan_bytes_from_local_storage);
+
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_remote_storage(
+ deltas.scan_bytes_from_remote_storage);
+
COUNTER_SET(_file_read_bytes_counter, bytes_read);
COUNTER_SET(_file_read_calls_counter,
cast_set<int64_t>(_file_reader_stats->read_calls));
COUNTER_SET(_file_read_time_counter,
cast_set<int64_t>(_file_reader_stats->read_time_ns));
+
+ DorisMetrics::instance()->query_scan_bytes->increment(deltas.scan_bytes);
+ DorisMetrics::instance()->query_scan_rows->increment(deltas.scan_rows);
+ DorisMetrics::instance()->query_scan_bytes_from_local->increment(
+ deltas.scan_bytes_from_local_storage);
+ DorisMetrics::instance()->query_scan_bytes_from_remote->increment(
+ deltas.scan_bytes_from_remote_storage);
+}
+
+FileScannerV2::RealtimeCounterDeltas
FileScannerV2::_collect_realtime_counter_deltas(
+ const io::FileReaderStats& file_reader_stats,
+ const io::FileCacheStatistics& file_cache_statistics, int64_t*
last_read_bytes,
+ int64_t* last_read_rows, int64_t* last_bytes_read_from_local,
+ int64_t* last_bytes_read_from_remote) {
+ DORIS_CHECK(last_read_bytes != nullptr);
+ DORIS_CHECK(last_read_rows != nullptr);
+ DORIS_CHECK(last_bytes_read_from_local != nullptr);
+ DORIS_CHECK(last_bytes_read_from_remote != nullptr);
+
+ const int64_t read_bytes = cast_set<int64_t>(file_reader_stats.read_bytes);
+ const int64_t read_rows = cast_set<int64_t>(file_reader_stats.read_rows);
+ const int64_t bytes_read_from_local =
file_cache_statistics.bytes_read_from_local;
+ const int64_t bytes_read_from_remote =
file_cache_statistics.bytes_read_from_remote;
+ DORIS_CHECK(read_bytes >= *last_read_bytes);
+ DORIS_CHECK(read_rows >= *last_read_rows);
+ DORIS_CHECK(bytes_read_from_local >= *last_bytes_read_from_local);
+ DORIS_CHECK(bytes_read_from_remote >= *last_bytes_read_from_remote);
+
+ RealtimeCounterDeltas deltas;
+ deltas.scan_rows = read_rows - *last_read_rows;
+ deltas.scan_bytes = read_bytes - *last_read_bytes;
+ if (bytes_read_from_local == 0 && bytes_read_from_remote == 0) {
Review Comment:
This fallback treats peer-cache-only reads as remote storage.
`CachedRemoteFileReader::_update_stats()` can populate
`bytes_read_from_peer`/`num_peer_io_total` while both `bytes_read_from_local`
and `bytes_read_from_remote` remain zero, but this branch only checks local and
remote before charging the full `read_bytes` delta to
`scan_bytes_from_remote_storage`. That makes a query served from peer cache
contribute to the workload policy counter that is specifically exposed as
remote-storage bytes. Please include the peer counters when deciding whether
cache stats are present, and then choose an explicit peer semantic rather than
falling back to all-remote.
--
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]