hudi-agent commented on code in PR #19293:
URL: https://github.com/apache/hudi/pull/19293#discussion_r3584477225
##########
hudi-common/src/main/java/org/apache/hudi/common/model/FileSlice.java:
##########
@@ -193,24 +196,42 @@ public boolean isEmpty() {
}
/**
- * Get the total file size of a file slice similar on the base file.
- * For the log file, we need to convert its size to the estimated size
similar on the base file in a certain proportion
+ * Gets the estimated total size of a file slice in the base Parquet format.
+ *
+ * <p>Only inline Avro log files need size calibration. Native log files and
inline non-Avro log files
+ * use their physical sizes directly.</p>
+ *
+ * @param writeConfig write config containing the log data block format and
log-to-Parquet compression ratio
*/
- public long getTotalFileSizeAsParquetFormat(double logFileFraction) {
- long logFileSize =
convertLogFilesSizeToExpectedParquetSize(logFileFraction);
+ public long getTotalFileSizeAsParquetFormat(HoodieConfig writeConfig) {
+ long logFileSize = convertLogFilesSizeToExpectedParquetSize(writeConfig);
return getBaseFile().isPresent() ? getBaseFile().get().getFileSize() +
logFileSize : logFileSize;
}
- private long convertLogFilesSizeToExpectedParquetSize(double
logFileFraction) {
- long totalSizeOfLogFiles =
+ private long convertLogFilesSizeToExpectedParquetSize(HoodieConfig
writeConfig) {
+ long totalSizeOfInlineLogFiles =
+ logFiles.stream()
+ .filter(logFile -> !logFile.isNativeLogFile())
+ .map(HoodieLogFile::getFileSize)
+ .filter(size -> size > 0)
+ .reduce(Long::sum)
+ .orElse(0L);
+ long totalSizeOfNativeLogFiles =
logFiles.stream()
+ .filter(HoodieLogFile::isNativeLogFile)
.map(HoodieLogFile::getFileSize)
.filter(size -> size > 0)
.reduce(Long::sum)
.orElse(0L);
- // Here we assume that if there is no base parquet file, all log files
contain only inserts.
+ // Here we assume that if there is no base parquet file, all calibrated
log files contain only inserts.
// We can then just get the parquet equivalent size of these log files,
compare that with
// {@link config.getParquetMaxFileSize()} and decide if there is scope to
insert more rows
- return (long) (totalSizeOfLogFiles * logFileFraction);
+ boolean isAvroDataBlocks =
HoodieLogBlockType.fromId(writeConfig.getStringOrDefault(
Review Comment:
🤖 Here `isAvroDataBlocks` is derived purely from `LOGFILE_DATA_BLOCK_FORMAT`
(defaulting to `avro`), but
`DeltaWriteProfile.logFileToParquetCompressionRatio` resolves the effective
type via `CommonClientUtils.getLogBlockType`, which also falls back to the
base-file format when that config is unset. For a table with the config unset
but a non-Avro effective block type (e.g. HFILE base format), this path would
still calibrate the inline logs while the Flink avg-size path would not. Is
that divergence intentional? (I realize hudi-common can't reach
`getLogBlockType` directly, so this may just be an accepted tradeoff worth a
comment.)
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/partitioner/profile/DeltaWriteProfile.java:
##########
@@ -123,10 +125,10 @@ private boolean isSmallFile(FileSlice fileSlice) {
}
private double logFileToParquetCompressionRatio() {
Review Comment:
🤖 nit: it looks like `logFileToParquetCompressionRatio()` is no longer
called after `getTotalFileSize` was updated to delegate to
`fileSlice.getTotalFileSizeAsParquetFormat(config)` directly — could this
private method be removed? Leaving it around risks confusing a future reader
about whether it represents the authoritative calibration logic or is just a
stale copy.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
