prashantwason commented on a change in pull request #2494:
URL: https://github.com/apache/hudi/pull/2494#discussion_r578824544
##########
File path:
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java
##########
@@ -188,41 +238,42 @@ private synchronized void openFileSliceIfNeeded() throws
IOException {
// Load the schema
Schema schema =
HoodieAvroUtils.addMetadataFields(HoodieMetadataRecord.getClassSchema());
- logRecordScanner = new
HoodieMetadataMergedLogRecordScanner(metaClient.getFs(), metadataBasePath,
- logFilePaths, schema, latestMetaInstantTimestamp,
MAX_MEMORY_SIZE_IN_BYTES, BUFFER_SIZE,
+ HoodieMetadataMergedLogRecordScanner logRecordScanner = new
HoodieMetadataMergedLogRecordScanner(metaClient.getFs(),
+ metadataBasePath, logFilePaths, schema,
latestMetaInstantTimestamp, MAX_MEMORY_SIZE_IN_BYTES, BUFFER_SIZE,
spillableMapDirectory, null);
- LOG.info("Opened metadata log files from " + logFilePaths + " at instant "
+ latestInstantTime
- + "(dataset instant=" + latestInstantTime + ", metadata instant=" +
latestMetaInstantTimestamp + ")");
+ timings[1] = timer.endTimer();
+ LOG.info(String.format("Opened metadata log files from %s at instant
(dataset instant=%s, metadata instant=%s) in %d ms",
+ logFilePaths, latestInstantTime, latestMetaInstantTimestamp,
timings[1]));
- metrics.ifPresent(metrics ->
metrics.updateMetrics(HoodieMetadataMetrics.SCAN_STR, timer.endTimer()));
+ metrics.ifPresent(metrics ->
metrics.updateMetrics(HoodieMetadataMetrics.SCAN_STR, timings[0] + timings[1]));
+ return Pair.of(baseFileReader, logRecordScanner);
}
- private void closeIfNeeded() {
+ private void close(HoodieFileReader localFileReader,
HoodieMetadataMergedLogRecordScanner localLogScanner) {
try {
- if (!metadataConfig.enableReuse()) {
- close();
+ if (localFileReader != null) {
+ localFileReader.close();
+ }
+ if (localLogScanner != null) {
+ localLogScanner.close();
}
} catch (Exception e) {
throw new HoodieException("Error closing resources during metadata table
merge", e);
}
}
@Override
- public void close() throws Exception {
- if (baseFileReader != null) {
- baseFileReader.close();
- baseFileReader = null;
- }
- if (logRecordScanner != null) {
- logRecordScanner.close();
- logRecordScanner = null;
- }
+ public synchronized void close() throws Exception {
+ close(baseFileReader, logRecordScanner);
+ baseFileReader = null;
Review comment:
The close(HoodieFileReader localFileReader,
HoodieMetadataMergedLogRecordScanner localLogScanner) does not touch the member
variables. It only closes the readers provided as the function argument.
In other words, how will the close(....) function know whether to set the
member variables to null? It should only set the member variables to null if
this is the final close (we also call close() after each
getRecordByKeyFromMetadata() when reuse=FALSE)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
