vinothchandar commented on a change in pull request #2417: URL: https://github.com/apache/hudi/pull/2417#discussion_r554259024
########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java ########## @@ -49,12 +60,48 @@ public FileSystemBackedTableMetadata(SerializableConfiguration conf, String data @Override public List<String> getAllPartitionPaths() throws IOException { - FileSystem fs = new Path(datasetBasePath).getFileSystem(hadoopConf.get()); if (assumeDatePartitioning) { + FileSystem fs = new Path(datasetBasePath).getFileSystem(hadoopConf.get()); return FSUtils.getAllPartitionFoldersThreeLevelsDown(fs, datasetBasePath); - } else { - return FSUtils.getAllFoldersWithPartitionMetaFile(fs, datasetBasePath); } + + List<Path> pathsToList = new LinkedList<>(); + pathsToList.add(new Path(datasetBasePath)); + List<String> partitionPaths = new ArrayList<>(); + + // TODO: Get the parallelism from HoodieWriteConfig + final int fileListingParallelism = 1500; Review comment: Okay my bad. can we pull this into a static final variable for now. ########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java ########## @@ -64,6 +111,6 @@ public FileSystemBackedTableMetadata(SerializableConfiguration conf, String data @Override public boolean isInSync() { - throw new UnsupportedOperationException(); + return false; Review comment: the stats are important, so let's keep it there. I think this happens because some parts of the tests run with metadata enabled and this is returned as the fallback. If its not too many, can we fix the tests. it does not make sense to have this return `false` semantically. ########## File path: hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java ########## @@ -49,12 +60,48 @@ public FileSystemBackedTableMetadata(SerializableConfiguration conf, String data @Override public List<String> getAllPartitionPaths() throws IOException { - FileSystem fs = new Path(datasetBasePath).getFileSystem(hadoopConf.get()); if (assumeDatePartitioning) { + FileSystem fs = new Path(datasetBasePath).getFileSystem(hadoopConf.get()); return FSUtils.getAllPartitionFoldersThreeLevelsDown(fs, datasetBasePath); - } else { - return FSUtils.getAllFoldersWithPartitionMetaFile(fs, datasetBasePath); } + + List<Path> pathsToList = new LinkedList<>(); Review comment: Can we add more tests to this piece of code? (previously it was just in the bootstrap part), not this is prime time. ########## File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/FileSystemViewManager.java ########## @@ -159,12 +160,12 @@ private static HoodieTableFileSystemView createInMemoryFileSystemView(Serializab return new HoodieTableFileSystemView(metaClient, timeline, viewConf.isIncrementalTimelineSyncEnabled()); } - public static HoodieTableFileSystemView createInMemoryFileSystemView(HoodieTableMetaClient metaClient, - boolean useFileListingFromMetadata, - boolean verifyListings) { + public static HoodieTableFileSystemView createInMemoryFileSystemViewForReaders(HoodieTableMetaClient metaClient, + boolean useFileListingFromMetadata, boolean verifyListings) { LOG.info("Creating InMemory based view for basePath " + metaClient.getBasePath()); if (useFileListingFromMetadata) { - return new HoodieMetadataFileSystemView(metaClient, + HoodieMREngineContext engineContext = new HoodieMREngineContext(metaClient.getHadoopConf()); Review comment: On a side note, we should move it to the hudi-hadoop-mr module or rename this to `HoodieLocalEngine` or something. Lets discuss the relocation in the other comment. ---------------------------------------------------------------- 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: us...@infra.apache.org