Copilot commented on code in PR #61072:
URL: https://github.com/apache/doris/pull/61072#discussion_r2894319292


##########
be/src/vec/exec/scan/olap_scanner.cpp:
##########
@@ -692,25 +692,31 @@ void OlapScanner::update_realtime_counters() {
                 stats.compressed_bytes_read);
     } else {
         
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_local_storage(
-                stats.file_cache_stats.bytes_read_from_local - 
_bytes_read_from_local);
+                stats.file_cache_stats.bytes_read_from_local);
         _state->get_query_ctx()
                 ->resource_ctx()
                 ->io_context()
                 ->update_scan_bytes_from_remote_storage(
-                        stats.file_cache_stats.bytes_read_from_remote - 
_bytes_read_from_remote);
+                        stats.file_cache_stats.bytes_read_from_remote);
 
         DorisMetrics::instance()->query_scan_bytes_from_local->increment(
-                stats.file_cache_stats.bytes_read_from_local - 
_bytes_read_from_local);
+                stats.file_cache_stats.bytes_read_from_local);
         DorisMetrics::instance()->query_scan_bytes_from_remote->increment(
-                stats.file_cache_stats.bytes_read_from_remote - 
_bytes_read_from_remote);
+                stats.file_cache_stats.bytes_read_from_remote);
+    }
+
+    if (config::is_cloud_mode() && config::enable_file_cache) {
+        io::FileCacheProfileReporter 
cache_profile(local_state->_segment_profile.get());
+        cache_profile.update(&stats.file_cache_stats);
     }
 
     _tablet_reader->mutable_stats()->compressed_bytes_read = 0;
     _tablet_reader->mutable_stats()->uncompressed_bytes_read = 0;
     _tablet_reader->mutable_stats()->raw_rows_read = 0;
+    _tablet_reader->mutable_stats()->file_cache_stats = {};

Review Comment:
   Bug: By resetting `file_cache_stats = {}` at line 716, the cumulative 
`bytes_write_into_cache` that was previously tracked across all cycles is now 
lost. In `_collect_profile_before_close()` (line 846–847), 
`update_bytes_write_into_cache` is called with 
`stats.file_cache_stats.bytes_write_into_cache`, but since `file_cache_stats` 
is now reset to `{}` every cycle, this will only report the bytes written since 
the last `update_realtime_counters()` call, not the total.
   
   To fix this, you should also call `update_bytes_write_into_cache` inside 
`update_realtime_counters` (in the `config::is_cloud_mode() && 
config::enable_file_cache` block), similar to how `cache_profile.update` is 
called here.



##########
be/src/vec/exec/scan/olap_scanner.cpp:
##########
@@ -692,25 +692,31 @@ void OlapScanner::update_realtime_counters() {
                 stats.compressed_bytes_read);
     } else {
         
_state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_local_storage(
-                stats.file_cache_stats.bytes_read_from_local - 
_bytes_read_from_local);
+                stats.file_cache_stats.bytes_read_from_local);
         _state->get_query_ctx()
                 ->resource_ctx()
                 ->io_context()
                 ->update_scan_bytes_from_remote_storage(
-                        stats.file_cache_stats.bytes_read_from_remote - 
_bytes_read_from_remote);
+                        stats.file_cache_stats.bytes_read_from_remote);
 
         DorisMetrics::instance()->query_scan_bytes_from_local->increment(
-                stats.file_cache_stats.bytes_read_from_local - 
_bytes_read_from_local);
+                stats.file_cache_stats.bytes_read_from_local);
         DorisMetrics::instance()->query_scan_bytes_from_remote->increment(
-                stats.file_cache_stats.bytes_read_from_remote - 
_bytes_read_from_remote);
+                stats.file_cache_stats.bytes_read_from_remote);
+    }
+
+    if (config::is_cloud_mode() && config::enable_file_cache) {
+        io::FileCacheProfileReporter 
cache_profile(local_state->_segment_profile.get());
+        cache_profile.update(&stats.file_cache_stats);
     }
 
     _tablet_reader->mutable_stats()->compressed_bytes_read = 0;
     _tablet_reader->mutable_stats()->uncompressed_bytes_read = 0;
     _tablet_reader->mutable_stats()->raw_rows_read = 0;
+    _tablet_reader->mutable_stats()->file_cache_stats = {};
 
-    _bytes_read_from_local = 
_tablet_reader->stats().file_cache_stats.bytes_read_from_local;
-    _bytes_read_from_remote = 
_tablet_reader->stats().file_cache_stats.bytes_read_from_remote;
+    _bytes_read_from_local = 0;
+    _bytes_read_from_remote = 0;

Review Comment:
   The member variables `_bytes_read_from_local` and `_bytes_read_from_remote` 
are now dead code. Since `file_cache_stats` is reset to `{}` at the end of each 
cycle, these variables are always set to 0 at lines 718-719 and never read 
before being re-assigned. They should be removed from both the `.cpp` and `.h` 
files to avoid confusion.



-- 
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