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.
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/engine/HoodieMREngineContext.java
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.common.engine;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hudi.common.config.SerializableConfiguration;
+import org.apache.hudi.common.function.SerializableConsumer;
+import org.apache.hudi.common.function.SerializableFunction;
+import org.apache.hudi.common.function.SerializablePairFunction;
+import org.apache.hudi.common.util.Option;
+
+import org.apache.hudi.common.util.collection.Pair;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.util.stream.Collectors.toList;
+import static
org.apache.hudi.common.function.FunctionWrapper.throwingFlatMapWrapper;
+import static
org.apache.hudi.common.function.FunctionWrapper.throwingForeachWrapper;
+import static
org.apache.hudi.common.function.FunctionWrapper.throwingMapToPairWrapper;
+import static
org.apache.hudi.common.function.FunctionWrapper.throwingMapWrapper;
+
+/**
+ * A java based engine context that can be used from map-reduce tasks
executing in query engines like
+ * spark, hive and presto.
+ */
+public final class HoodieMREngineContext extends HoodieEngineContext {
Review comment:
> But we should not get rid of 2 because we are treating
HoodieEngineContext as the default engine context to instantiate
But that only depends on the interface right. `hudi-cli` has access to
sparkContext right?
----------------------------------------------------------------
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
