aokolnychyi commented on code in PR #8263:
URL: https://github.com/apache/iceberg/pull/8263#discussion_r1288723879
##########
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 wasn't sure myself. It felt to me a few similar methods like this would be
appropriate to hide the complexity of populating the same values from multiple
places. If we go with this one, I'd expect more methods. For instance, we could
have one that accepts a matching data file and updates both the count as well
as the total file size. A similar thing can be done for the array of delete
files, etc.
I see a few potential benefits:
- Less metrics related code (1 line vs 14 lines in the delete index), moving
the complexity to `ScanMetrics`.
- Methods can be shared (e.g. distributed planning updates exactly the same
metrics as `ManifestGroup`).
- We can add more metrics implicitly without requiring changes outside of
`ScanMetrics`.
That said, I don't feel strongly. How did you envision this API, @nastra?
--
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]
