nastra commented on code in PR #8263:
URL: https://github.com/apache/iceberg/pull/8263#discussion_r1290159823
##########
core/src/main/java/org/apache/iceberg/metrics/ScanMetrics.java:
##########
@@ -127,6 +129,16 @@ public Counter positionalDeleteFiles() {
return metricsContext().counter(POSITIONAL_DELETE_FILES);
}
+ public void indexedDeleteFile(DeleteFile deleteFile) {
Review Comment:
I gave this some thought. I think we shouldn't add complexity to the
`ScanMetrics` API itself in order to populate more than one metric. That just
adds additional complexity to the API and all of a sudden also adds
non-metrcis-related dependencies to it (like `DeleteFile` and such).
The purpose of `ScanMetrics` is to just be a data container for scan-related
metrics, that are populated during scan planning, so I think it would be better
to keep this API clean.
On the other hand I understand that we want to reduce metrics-related stuff
in the code itself and I think we should maybe just have a **util class** that
handles this? This would allow us to keep `ScanMetrics` clean but also extract
metrics-related code that calls the right increments depending on a particular
file type in one single place.
--
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]
