morningman commented on code in PR #68713:
URL: https://github.com/apache/doris/pull/68713#discussion_r4396300892
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -2219,6 +2239,47 @@ && containsVariant(field.type()))) {
return physicalVariantSchemaIds;
}
+ /**
+ * What the JNI splits of {@code table} declare (PaimonJniHeapEstimate). A
file holds row groups of the
+ * size set by the schema version it was written under, which an ALTER
TABLE since leaves as it is, so
+ * the estimate reads the versions its files were written under, of the
branch that planned its split: a
+ * scan.fallback-branch or scan.primary-branch table plans the splits of
two branches, each numbering its
+ * schema versions on its own. $ro, $audit_log, $binlog and $row_tracking
read their base table's files,
+ * under that table's schema ids, while showing its current options; the
metadata system tables are
+ * estimated by the options they show.
+ */
+ private ToLongFunction<Split> jniHeapBytes(Table table, PaimonTableHandle
handle) {
+ // The base table of a wrapper, resolved as the timestamp-history
checks resolve it.
+ Table physicalSchemaTable = withSchemaHistoryAuthentication(() ->
resolveTimestampHistoryTable(table, handle));
+ if (!(physicalSchemaTable instanceof FileStoreTable)) {
+ return PaimonJniHeapEstimate.of(table)::bytesOf;
+ }
+ FileStoreTable fileStoreTable = (FileStoreTable) physicalSchemaTable;
+ Map<FileStoreTable, PaimonJniHeapEstimate> branchEstimates = new
java.util.IdentityHashMap<>();
+ return split ->
branchEstimates.computeIfAbsent(splitBranch(fileStoreTable, split),
this::jniHeapEstimate)
Review Comment:
Fixed in 7e152488295.
**Confirmed.** `AuditLogRead` calls `forceKeepDelete()` (`BinlogRead`
extends it), and `PrimaryKeyTableRawFileSplitReadProvider.match` requires
`!context.forceKeepDelete()`, so under `$audit_log` and `$binlog` every
DataSplit of a primary-key table goes to
`MergeFileSplitRead#createMergeReader`. A deletion-vector table hits this more
often than a first-row one: `MergeTreeSplitGenerator#splitForBatch` packs files
of any number of levels into a raw-convertible split for both.
**One correction to the framing.** `jniHeapBytes()` did not lose the read
mode when it resolved the wrapper to its physical table; the estimate never had
it. Before f6e44169062 these wrappers were estimated through
`PaimonJniHeapEstimate.of(Table)`, and `AuditLogTable.primaryKeys()` is empty,
so they were taken as append tables and declared their largest file as well.
**What changed.** The estimate now knows whether the read keeps deletes (the
wrapper is an `AuditLogTable`). For such a read, a primary-key table's
raw-convertible split is declared as merged when its files are on more than one
level. A split of one level still declares its largest file: a level is one
sorted run whose keys do not overlap, so `IntervalPartition` gives each file a
section of its own and the merge reads them one after another. Summing every
split would declare the `$audit_log` splits of a compacted table several times
over: four 32 MB files of a 20-column table would go from about 55 MB to about
220 MB.
**Tests.**
-
`PaimonJniHeapEstimateTest.readThatKeepsDeletesMergesTheFilesOfMoreThanOneLevel`:
a split of files on levels 5 and 4 declares the larger file for a plain read
and both for a read that keeps deletes; a one-level split declares the larger
file either way.
-
`PaimonScanPlanProviderTest.auditLogAndBinlogDeclareEveryFileOfTheSplitsTheyMerge`:
a deletion-vector table whose update is compacted onto level 4, over the
level-5 file of the first write. Read through JNI, the table declares the
larger file; `$audit_log` and `$binlog` declare both.
- Both fail without the fix. fe-connector-paimon (903 tests) and
fe-connector-fluss (296) pass.
--
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]
