yihua commented on code in PR #18412:
URL: https://github.com/apache/hudi/pull/18412#discussion_r3036400813
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieLogRecordReader.java:
##########
@@ -450,6 +484,24 @@ private void
processQueuedBlocksForInstant(Deque<HoodieLogBlock> logBlocks, int
progress = (float) (numLogFilesSeen - 1) / logFiles.size();
}
+ private void processDataBlock(HoodieDataBlock dataBlock, Option<KeySpec>
keySpecOpt) throws IOException {
+ String blockInstantTime = dataBlock.getLogBlockHeader().get(INSTANT_TIME);
+ LOG.debug("Processing log block with instant time {}", blockInstantTime);
+ long totalLogRecordsBefore = recordBuffer != null ?
recordBuffer.getTotalLogRecords() : 0L;
+ HoodieTimer blockReadTimer = HoodieTimer.start();
+ recordBuffer.processDataBlock(dataBlock, keySpecOpt);
+ long recordsInBlock = recordBuffer != null ?
recordBuffer.getTotalLogRecords() - totalLogRecordsBefore : 0L;
+
+ Map<String, Object> blockReadMetrics = new HashMap<>();
+ blockReadMetrics.put(LOG_BLOCK_FULL_READ_DURATION_IN_MILLIS,
blockReadTimer.endTimer());
+ blockReadMetrics.put(TOTAL_RECORDS_PRESENT_IN_LOG_BLOCK, recordsInBlock);
+ dataBlock.getBlockContentLocation()
+ .map(contentLocation -> blockReadMetrics.put(BLOCK_SIZE_IN_BYTES,
contentLocation.getBlockSize()));
+
blockReadMetrics.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME.toString(),
blockInstantTime);
+ blocksStats.add(blockReadMetrics);
+ LOG.debug("For log block, scan metrics are {}", blockReadMetrics);
Review Comment:
🤖 nit: could use `.ifPresent()` instead of `.map()` here too for the side
effect — clearer intent and avoids the unused Optional.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieLogRecordReader.java:
##########
@@ -253,7 +272,10 @@ protected final synchronized void
scanInternal(Option<KeySpec> keySpecOpt, boole
totalLogFiles.set(scannedLogFiles.size());
// Use the HoodieLogFileReader to iterate through the blocks in the
log file
HoodieLogBlock logBlock = logFormatReaderWrapper.next();
Review Comment:
🤖 nit: it might be worth using `.ifPresent()` instead of `.map()` here for
side effects — `.ifPresent()` is more idiomatic and avoids unused Optional
warnings.
--
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]
