rmpifer commented on a change in pull request #2342: URL: https://github.com/apache/hudi/pull/2342#discussion_r547573663
########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataMergedInstantRecordScanner.java ########## @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metadata; + +import org.apache.avro.Schema; +import org.apache.hudi.common.model.HoodieKey; +import org.apache.hudi.common.model.HoodieRecord; +import org.apache.hudi.common.model.HoodieRecordPayload; +import org.apache.hudi.common.table.HoodieTableMetaClient; +import org.apache.hudi.common.table.timeline.HoodieInstant; +import org.apache.hudi.common.util.DefaultSizeEstimator; +import org.apache.hudi.common.util.HoodieRecordSizeEstimator; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.common.util.collection.ExternalSpillableMap; +import org.apache.hudi.exception.HoodieException; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * Provides functionality to convert timeline instants to table metadata records and then merge by key. Specify + * a filter to limit keys that are merged and stored in memory. + */ +public class HoodieMetadataMergedInstantRecordScanner { + + private static final Logger LOG = LogManager.getLogger(HoodieMetadataMergedInstantRecordScanner.class); + + HoodieTableMetaClient metaClient; + private List<HoodieInstant> instants; + private Set<String> mergeKeyFilter; + protected final ExternalSpillableMap<String, HoodieRecord<? extends HoodieRecordPayload>> records; + + public HoodieMetadataMergedInstantRecordScanner(HoodieTableMetaClient metaClient, List<HoodieInstant> instants, + Schema readerSchema, Long maxMemorySizeInBytes, + String spillableMapBasePath, Set<String> mergeKeyFilter) throws IOException { + this.metaClient = metaClient; + this.instants = instants; + this.mergeKeyFilter = mergeKeyFilter != null ? mergeKeyFilter : Collections.emptySet(); + this.records = new ExternalSpillableMap<>(maxMemorySizeInBytes, spillableMapBasePath, new DefaultSizeEstimator(), + new HoodieRecordSizeEstimator(readerSchema)); + + scan(); + } + + private void scan() { + for (HoodieInstant instant : instants) { + try { + processInstant(instant); + } catch (Exception e) { + LOG.error(String.format("Got exception when processing timeline instant %s", instant.getTimestamp()), e); + throw new HoodieException(String.format("Got exception when processing timeline instant %s", instant.getTimestamp()), e); + } + } + } + + /** + * Converts an instant to metadata table records and processes each record. + * + * @param instant + * @throws IOException + */ + private void processInstant(HoodieInstant instant) throws IOException { Review comment: Updated ########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java ########## @@ -372,19 +191,6 @@ protected void closeReaders() { logRecordScanner = null; } Review comment: Thanks for the callout. I added this to `closeReaders()` ########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/AbstractHoodieTableMetadata.java ########## @@ -0,0 +1,296 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metadata; + +import org.apache.avro.Schema; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hudi.avro.HoodieAvroUtils; +import org.apache.hudi.avro.model.HoodieMetadataRecord; +import org.apache.hudi.common.config.SerializableConfiguration; +import org.apache.hudi.common.fs.FSUtils; +import org.apache.hudi.common.metrics.Registry; +import org.apache.hudi.common.model.HoodiePartitionMetadata; +import org.apache.hudi.common.model.HoodieRecord; +import org.apache.hudi.common.model.HoodieRecordPayload; +import org.apache.hudi.common.table.HoodieTableMetaClient; +import org.apache.hudi.common.table.timeline.HoodieInstant; +import org.apache.hudi.common.util.HoodieTimer; +import org.apache.hudi.common.util.Option; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.Path; +import org.apache.hudi.exception.HoodieMetadataException; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Interface that supports querying various pieces of metadata about a hudi table. + */ +public abstract class AbstractHoodieTableMetadata implements HoodieTableMetadata { + + private static final Logger LOG = LogManager.getLogger(AbstractHoodieTableMetadata.class); + + static final long MAX_MEMORY_SIZE_IN_BYTES = 1024 * 1024 * 1024; + static final int BUFFER_SIZE = 10 * 1024 * 1024; + + protected final SerializableConfiguration hadoopConf; + protected final Option<HoodieMetadataMetrics> metrics; + protected final String datasetBasePath; + + // Directory used for Spillable Map when merging records + final String spillableMapDirectory; + + protected boolean enabled; + private final boolean validateLookups; + private final boolean assumeDatePartitioning; + + private transient HoodieMetadataMergedInstantRecordScanner timelineRecordScanner; + + protected AbstractHoodieTableMetadata(Configuration hadoopConf, String datasetBasePath, String spillableMapDirectory, + boolean enabled, boolean validateLookups, boolean enableMetrics, + boolean assumeDatePartitioning) { + this.hadoopConf = new SerializableConfiguration(hadoopConf); + this.datasetBasePath = datasetBasePath; + this.spillableMapDirectory = spillableMapDirectory; + + this.enabled = enabled; + this.validateLookups = validateLookups; + this.assumeDatePartitioning = assumeDatePartitioning; + + if (enableMetrics) { + this.metrics = Option.of(new HoodieMetadataMetrics(Registry.getRegistry("HoodieMetadata"))); + } else { + this.metrics = Option.empty(); + } + } + + public static AbstractHoodieTableMetadata create(Configuration conf, String datasetBasePath, String spillableMapPath, boolean useFileListingFromMetadata, + boolean verifyListings, boolean enableMetrics, boolean shouldAssumeDatePartitioning) { + return new HoodieBackedTableMetadata(conf, datasetBasePath, spillableMapPath, useFileListingFromMetadata, verifyListings, + enableMetrics, shouldAssumeDatePartitioning); + } + + + /** + * Return the list of files in a partition. + * + * If the Metadata Table is enabled, the listing is retrieved from the stored metadata. Otherwise, the list of + * partitions is retrieved directly from the underlying {@code FileSystem}. + * + * On any errors retrieving the listing from the metadata, defaults to using the file system listings. + * + * @param partitionPath The absolute path of the partition to list + */ + + public FileStatus[] getAllFilesInPartition(Path partitionPath) throws IOException { + if (enabled) { + try { + return fetchAllFilesInPartition(partitionPath); + } catch (Exception e) { + LOG.error("Failed to retrive files in partition " + partitionPath + " from metadata", e); + } + } + return FSUtils.getFs(partitionPath.toString(), hadoopConf.get()).listStatus(partitionPath); + } + + /** + * Return the list of partitions in the dataset. + * + * If the Metadata Table is enabled, the listing is retrieved from the stored metadata. Otherwise, the list of + * partitions is retrieved directly from the underlying {@code FileSystem}. + * + * On any errors retrieving the listing from the metadata, defaults to using the file system listings. + * + */ + public List<String> getAllPartitionPaths() throws IOException { + if (enabled) { + try { + return fetchAllPartitionPaths(); + } catch (Exception e) { + LOG.error("Failed to retrieve list of partition from metadata", e); + } + } + + FileSystem fs = FSUtils.getFs(datasetBasePath, hadoopConf.get()); + return FSUtils.getAllPartitionPaths(fs, datasetBasePath, assumeDatePartitioning); + } + + /** + * Returns a list of all partitions. + */ + /** + * Returns a list of all partitions. + */ + protected List<String> fetchAllPartitionPaths() throws IOException { + HoodieTimer timer = new HoodieTimer().startTimer(); + Option<HoodieRecord<HoodieMetadataPayload>> hoodieRecord = getMergedRecordByKey(RECORDKEY_PARTITION_LIST); + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.LOOKUP_PARTITIONS_STR, timer.endTimer())); + + List<String> partitions = Collections.emptyList(); + if (hoodieRecord.isPresent()) { + if (!hoodieRecord.get().getData().getDeletions().isEmpty()) { + throw new HoodieMetadataException("Metadata partition list record is inconsistent: " + + hoodieRecord.get().getData()); + } + + partitions = hoodieRecord.get().getData().getFilenames(); + // Partition-less tables have a single empty partition + if (partitions.contains(NON_PARTITIONED_NAME)) { + partitions.remove(NON_PARTITIONED_NAME); + partitions.add(""); + } + } + + if (validateLookups) { + // Validate the Metadata Table data by listing the partitions from the file system + timer.startTimer(); + HoodieTableMetaClient metaClient = new HoodieTableMetaClient(hadoopConf.get(), datasetBasePath); + List<String> actualPartitions = FSUtils.getAllPartitionPaths(metaClient.getFs(), datasetBasePath, false); + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.VALIDATE_PARTITIONS_STR, timer.endTimer())); + + Collections.sort(actualPartitions); + Collections.sort(partitions); + if (!actualPartitions.equals(partitions)) { + LOG.error("Validation of metadata partition list failed. Lists do not match."); + LOG.error("Partitions from metadata: " + Arrays.toString(partitions.toArray())); + LOG.error("Partitions from file system: " + Arrays.toString(actualPartitions.toArray())); + + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.VALIDATE_ERRORS_STR, 0)); + } + + // Return the direct listing as it should be correct + partitions = actualPartitions; + } + + LOG.info("Listed partitions from metadata: #partitions=" + partitions.size()); + return partitions; + } + + /** + * Return all the files from the partition. + * + * @param partitionPath The absolute path of the partition + */ + private FileStatus[] fetchAllFilesInPartition(Path partitionPath) throws IOException { + String partitionName = FSUtils.getRelativePartitionPath(new Path(datasetBasePath), partitionPath); + if (partitionName.isEmpty()) { + partitionName = NON_PARTITIONED_NAME; + } + + HoodieTimer timer = new HoodieTimer().startTimer(); + Option<HoodieRecord<HoodieMetadataPayload>> hoodieRecord = getMergedRecordByKey(partitionName); + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.LOOKUP_FILES_STR, timer.endTimer())); + + FileStatus[] statuses = {}; + if (hoodieRecord.isPresent()) { + statuses = hoodieRecord.get().getData().getFileStatuses(partitionPath); + } + + if (validateLookups) { + // Validate the Metadata Table data by listing the partitions from the file system + timer.startTimer(); + + // Ignore partition metadata file + HoodieTableMetaClient metaClient = new HoodieTableMetaClient(hadoopConf.get(), datasetBasePath); + FileStatus[] directStatuses = metaClient.getFs().listStatus(partitionPath, + p -> !p.getName().equals(HoodiePartitionMetadata.HOODIE_PARTITION_METAFILE)); + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.VALIDATE_FILES_STR, timer.endTimer())); + + List<String> directFilenames = Arrays.stream(directStatuses) + .map(s -> s.getPath().getName()).sorted() + .collect(Collectors.toList()); + + List<String> metadataFilenames = Arrays.stream(statuses) + .map(s -> s.getPath().getName()).sorted() + .collect(Collectors.toList()); + + if (!metadataFilenames.equals(directFilenames)) { + LOG.error("Validation of metadata file listing for partition " + partitionName + " failed."); + LOG.error("File list from metadata: " + Arrays.toString(metadataFilenames.toArray())); + LOG.error("File list from direct listing: " + Arrays.toString(directFilenames.toArray())); + + metrics.ifPresent(m -> m.updateMetrics(HoodieMetadataMetrics.VALIDATE_ERRORS_STR, 0)); + } + + // Return the direct listing as it should be correct + statuses = directStatuses; + } + + LOG.info("Listed file in partition from metadata: partition=" + partitionName + ", #files=" + statuses.length); + return statuses; + } + + /** + * Retrieve the merged {@code HoodieRecord} mapped to the given key. + * + * @param key The key of the record + */ + private Option<HoodieRecord<HoodieMetadataPayload>> getMergedRecordByKey(String key) throws IOException { + openTimelineScanner(); + + Option<HoodieRecord<HoodieMetadataPayload>> metadataHoodieRecord = getRecordByKeyFromMetadata(key); + + // Retrieve record from unsynced timeline instants + Option<HoodieRecord<HoodieMetadataPayload>> timelineHoodieRecord = timelineRecordScanner.getRecordByKey(key); + if (timelineHoodieRecord.isPresent()) { + if (metadataHoodieRecord.isPresent()) { + HoodieRecordPayload mergedPayload = timelineHoodieRecord.get().getData().preCombine(metadataHoodieRecord.get().getData()); + metadataHoodieRecord = Option.of(new HoodieRecord(metadataHoodieRecord.get().getKey(), mergedPayload)); + } else { + metadataHoodieRecord = timelineHoodieRecord; Review comment: Sorry my comment was unclear. I created separate `mergedRecord` variable so I am not overriding `metadataHoodieRecord` ########## File path: hudi-client/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java ########## @@ -266,7 +256,7 @@ private void initialize(JavaSparkContext jsc, HoodieTableMetaClient datasetMetaC } private void initTableMetadata() { - this.metadata = new HoodieBackedTableMetadata(hadoopConf.get(), datasetWriteConfig.getBasePath(), datasetWriteConfig.getSpillableMapBasePath(), + this.metadata = (HoodieBackedTableMetadata) AbstractHoodieTableMetadata.create(hadoopConf.get(), datasetWriteConfig.getBasePath(), datasetWriteConfig.getSpillableMapBasePath(), Review comment: Along with Prashant's comment we can just create `HoodieBackedTableMetadata` since `HoodieBackedTableMetadaWriter` should only be associated with this implementation ########## File path: hudi-client/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java ########## @@ -266,7 +256,7 @@ private void initialize(JavaSparkContext jsc, HoodieTableMetaClient datasetMetaC } private void initTableMetadata() { - this.metadata = new HoodieBackedTableMetadata(hadoopConf.get(), datasetWriteConfig.getBasePath(), datasetWriteConfig.getSpillableMapBasePath(), Review comment: Agreed. Updated this ---------------------------------------------------------------- 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]
