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]

Reply via email to