blackmwk commented on code in PR #3283:
URL: https://github.com/apache/iceberg-rust/pull/3283#discussion_r4452543335
##########
crates/iceberg/src/arrow/scan_metrics.rs:
##########
@@ -52,23 +52,74 @@ impl<F: FileRead> FileRead for CountingFileRead<F> {
/// Metrics collected during an Iceberg scan.
#[derive(Clone, Debug)]
pub struct ScanMetrics {
- bytes_read: Arc<AtomicU64>,
+ data_bytes_read: Arc<AtomicU64>,
+ delete_bytes_read: Arc<AtomicU64>,
+ data_files_opened: Arc<AtomicU64>,
+ delete_files_opened: Arc<AtomicU64>,
+ rows_emitted: Arc<AtomicU64>,
Review Comment:
Seems the clone is everywhere, could you have a try with that direction? If
we go with this direction, we need to remove the `Clone` of `ScanMetrics`,
which is wasteful, but it may introduce a lot of changes. I want to see what it
looks like if we pass `ScanMetrics` around.
##########
crates/iceberg/src/arrow/reader/pipeline.rs:
##########
@@ -659,13 +663,21 @@ impl FileScanTaskReader {
// batches (using the reader-produced `_pos` position) before the
transformer, which
// then passes it through as a virtual field.
let first_row_id = task.first_row_id();
+ let scan_metrics = self.scan_metrics.clone();
Review Comment:
You only need to clone counter here.
##########
crates/iceberg/src/arrow/reader/pipeline.rs:
##########
@@ -167,7 +167,7 @@ impl FileScanTaskReader {
let path = task.data_file_path().to_string();
let file_io = self.file_io.clone();
let file_size_in_bytes = task.file_size_in_bytes();
- let bytes_read = Arc::clone(self.scan_metrics.bytes_read_counter());
+ let bytes_read =
Arc::clone(self.scan_metrics.data_bytes_read_counter());
Review Comment:
```suggestion
let data_file_bytes_read =
Arc::clone(self.scan_metrics.data_bytes_read_counter());
```
--
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]
