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:
[email protected]
