Skip to content

[HUDI-2779] Cache BaseDir if HudiTableNotFound Exception thrown #4014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
codope merged 5 commits into from
Dec 9, 2021
Merged

[HUDI-2779] Cache BaseDir if HudiTableNotFound Exception thrown #4014

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5fa4b10
Cache BaseDir if tableNotFound
Nov 17, 2021
476cd38
Add ut
Nov 24, 2021
a752e0a
Remove unnecessary changes
Nov 24, 2021
d3de6ff
Fix checkstyle and comment
Dec 1, 2021
ed0d854
Remove unnecessary changes
Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.apache.hudi.hadoop;

import com.google.common.annotations.VisibleForTesting;
import org.apache.hudi.common.config.SerializableConfiguration;
import org.apache.hudi.common.engine.HoodieLocalEngineContext;
import org.apache.hudi.common.fs.FSUtils;
Expand Down Expand Up @@ -173,6 +174,13 @@ public boolean accept(Path path) {
}

if (baseDir != null) {
// Check whether baseDir in nonHoodiePathCache
if (nonHoodiePathCache.contains(baseDir.toString())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Accepting non-hoodie path from cache: " + path);
}
return true;
}
HoodieTableFileSystemView fsView = null;
try {
HoodieTableMetaClient metaClient = metaClientCache.get(baseDir.toString());
Expand Down Expand Up @@ -211,9 +219,10 @@ public boolean accept(Path path) {
} catch (TableNotFoundException e) {
// Non-hoodie path, accept it.
if (LOG.isDebugEnabled()) {
LOG.debug(String.format("(1) Caching non-hoodie path under %s \n", folder.toString()));
LOG.debug(String.format("(1) Caching non-hoodie path under %s and %s \n", folder.toString(), baseDir.toString()));
Copy link
Member

@codope codope Nov 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path under %s and %s might confuse users. Maybe, path under %s with basePath %s would be more clear

}
nonHoodiePathCache.add(folder.toString());
nonHoodiePathCache.add(baseDir.toString());
return true;
}
} else {
Expand All @@ -231,6 +240,12 @@ public boolean accept(Path path) {
}
}

@VisibleForTesting
Copy link
Contributor Author

@boneanxs boneanxs Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As CheckStyle doesn't encourage to import com.google.common.annotations.VisibleForTesting annotation. I simply follow metaClientCache to make nonHoodiePathCache access specifier is friendly for test purpose.

public Set<String> getNonHoodiePathCache() {
return nonHoodiePathCache;
}


@Override
public void setConf(Configuration conf) {
this.conf = new SerializableConfiguration(conf);
Expand Down
3 changes: 3 additions & 0 deletions hudi-hadoop-mr/src/test/java/org/apache/hudi/hadoop/TestHoodieROTablePathFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public void testHoodiePaths() throws Exception {
assertFalse(pathFilter.accept(new Path("file:///" + basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME)));

assertEquals(1, pathFilter.metaClientCache.size());
assertEquals(0, pathFilter.getNonHoodiePathCache().size(), "NonHoodiePathCache size should be 0");
}

@Test
Expand All @@ -82,6 +83,7 @@ public void testNonHoodiePaths() throws IOException {
java.nio.file.Path path2 = Paths.get(basePath, "nonhoodiefolder/somefile");
Files.createFile(path2);
assertTrue(pathFilter.accept(new Path(path2.toUri())));
assertEquals(2, pathFilter.getNonHoodiePathCache().size(), "NonHoodiePathCache size should be 2");
}

@Test
Expand All @@ -93,5 +95,6 @@ public void testPartitionPathsAsNonHoodiePaths() throws Exception {
Path partitionPath2 = testTable.getPartitionPath(p2).getParent();
assertTrue(pathFilter.accept(partitionPath1), "Directories should be accepted");
assertTrue(pathFilter.accept(partitionPath2), "Directories should be accepted");
assertEquals(2, pathFilter.getNonHoodiePathCache().size(), "NonHoodiePathCache size should be 2");
}
}