Perseus commented on code in PR #3283:
URL: https://github.com/apache/iceberg-rust/pull/3283#discussion_r4446402769
##########
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:
👋 Thank you for the review!
I had started with this initially, I left a comment about why I didn't
pursue this in my PR description
> I'd initially started with moving all metrics to an inner field in
ScanMetrics and passing the entirety of ScanMetrics around, but when it came to
distinguishing between delete-file metrics and data-file metrics, it required
passing context about the type of file (data/delete/...) down into the
ArrowReader that I felt like would cause unnecessary coupling.
If the passing of the type of file into the ArrowReader is okay, I can make
this change
--
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]
