Core: Adapt commit, scan, and snapshot stats for DVs #11464
Conversation
| this.addedDVs += 1; | ||
| } else { | ||
| this.addedPosDeleteFiles += 1; | ||
| } | ||
| this.addedDeleteFiles += 1; |
There was a problem hiding this comment.
I am treating "delete files" as a generic concept. We will continue to track each type of delete as a delete file and will count DVs, position delete files, and equality delete files separately. I think this is consistent and won't require updating our manifest counts to differentiate between DVs and delete files.
There was a problem hiding this comment.
I agree that this is consistent
| long addedFileSize1 = dv2.contentSizeInBytes(); | ||
| long totalPosDeletes1 = dv1.recordCount() + dv2.recordCount(); | ||
| long totalFileSize1 = dv1.contentSizeInBytes() + dv2.contentSizeInBytes(); | ||
| assertThat(summary1) |
There was a problem hiding this comment.
It would be good to do https://github.com/apache/iceberg/pull/11302/files#r1824246937
There was a problem hiding this comment.
Missed, will do.
There was a problem hiding this comment.
Updated to cover all values.
| @@ -31,7 +32,11 @@ public static void indexedDeleteFile(ScanMetrics metrics, DeleteFile deleteFile) | |||
| metrics.indexedDeleteFiles().increment(); | |||
|
|
|||
| if (deleteFile.content() == FileContent.POSITION_DELETES) { | |||
| metrics.positionalDeleteFiles().increment(); | |||
| if (ContentFileUtil.isDV(deleteFile)) { | |||
There was a problem hiding this comment.
should this be using ScanTaskUtil.isDV() now?
There was a problem hiding this comment.
Replied with context here.
| @@ -84,4 +85,8 @@ public static String referencedDataFileLocation(DeleteFile deleteFile) { | |||
| CharSequence location = referencedDataFile(deleteFile); | |||
| return location != null ? location.toString() : null; | |||
| } | |||
|
|
|||
| public static boolean isDV(DeleteFile deleteFile) { | |||
There was a problem hiding this comment.
this isn't probably needed anymore, since the same exists in ScanTaskUtil
aokolnychyi
force-pushed
the
adapt-metrics-dvs
branch
from
587c981 to
c64b659
Compare
|
Thank you, @nastra! |
This PR adapts commit, scan, and snapshot stats for DVs.
This work is part of #11122.