JingsongLi commented on code in PR #9657:
URL: https://github.com/apache/paimon/pull/9657#discussion_r3986217707
##########
paimon-core/src/main/java/org/apache/paimon/operation/AbstractFileStoreScan.java:
##########
@@ -319,13 +320,26 @@ public Plan plan() {
manifestsResult.allManifests.stream()
.mapToLong(f -> f.numAddedFiles() -
f.numDeletedFiles())
.sum();
+ // for DELTA and CHANGELOG scan modes the result contains both ADD
and DELETE entries,
+ // only ADD entries will actually be read, so size and record
count only count them
+ long resultedTableFilesSize = 0L;
+ long resultedRecordCount = 0L;
+ for (ManifestEntry entry : result) {
+ if (entry.kind() == FileKind.ADD) {
+ resultedTableFilesSize += entry.file().fileSize();
Review Comment:
[P2] Include before-files when reporting the cost of change reads
The assumption that DELETE entries are never read is false for
SnapshotReaderImpl.readChanges(): it uses this DELTA plan's DELETE entries as
IncrementalSplit.beforeFiles, and
IncrementalDiffSplitRead/IncrementalChangelogReadProvider actually read those
files to produce changes. FollowUpScanner reaches this path for overwrite
changes, and AllDeltaFollowUpScanner uses it for delta reads. A deletion-only
change can therefore report zero resulted bytes/records while scanning the old
files; a replacement underreports the before side.
Keep the metrics' documented meaning tied to the files the consumer will
actually read, for example by calculating them after the normal-read versus
change-read split selection. Do not unconditionally sum all DELETE entries for
ordinary ADD-only reads. Add a DELTA/overwrite-changelog regression alongside
the existing batch gauge tests.
--
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]
