This is an automated email from the ASF dual-hosted git repository. voonhous pushed a commit to branch release-1.2.1 in repository https://gitbox.apache.org/repos/asf/hudi.git
commit e2467a4f57e86d2d3145f635dc359da476d59c49 Author: Sivabalan Narayanan <[email protected]> AuthorDate: Fri Jun 26 16:46:07 2026 -0700 fix: Skip missing properties files gracefully in DFSPropertiesConfiguration (#18805) * fix: Skip missing properties files gracefully in DFSPropertiesConfiguration DFSPropertiesConfiguration.addPropsFromFile only checked file existence when the path equalled DEFAULT_PATH. For any other path (e.g. an include= directive pointing at a non-existent file), the writer would proceed to open() and surface a FileNotFoundException, breaking the load even though other includes and inline properties were valid. Remove the DEFAULT_PATH gate so a missing file is logged and skipped regardless of which path triggered the call. The behavior already documented in the existing log message ("Properties file ... not found. Ignoring to load props file") now applies uniformly. --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> (cherry picked from commit aafbb638e3be6014ff710c03c3ff7869954007fb) --- .../common/config/DFSPropertiesConfiguration.java | 44 +++++++++--- .../util/TestDFSPropertiesConfiguration.java | 83 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 8 deletions(-) diff --git a/hudi-hadoop-common/src/main/java/org/apache/hudi/common/config/DFSPropertiesConfiguration.java b/hudi-hadoop-common/src/main/java/org/apache/hudi/common/config/DFSPropertiesConfiguration.java index 999cfdfed2af..daabcf41dbb0 100644 --- a/hudi-hadoop-common/src/main/java/org/apache/hudi/common/config/DFSPropertiesConfiguration.java +++ b/hudi-hadoop-common/src/main/java/org/apache/hudi/common/config/DFSPropertiesConfiguration.java @@ -118,15 +118,18 @@ public class DFSPropertiesConfiguration extends PropertiesConfig { String.format("Failed to read %s from class loader", DEFAULT_PROPERTIES_FILE), ioe); } } - // Try loading the external config file from local file system + // Try loading the external config file from local file system. Both DEFAULT_PATH and + // HUDI_CONF_DIR are optional global config locations — use the tolerant overload so a + // missing file does not propagate as an exception (preserves prior silent-ignore behavior + // for optional global-defaults paths). try { - conf.addPropsFromFile(DEFAULT_PATH); + conf.addPropsFromFile(DEFAULT_PATH, true); } catch (Exception e) { log.warn("Cannot load default config file: {}", DEFAULT_PATH, e); } Option<StoragePath> defaultConfPath = getConfPathFromEnv(); if (defaultConfPath.isPresent() && !defaultConfPath.get().equals(DEFAULT_PATH)) { - conf.addPropsFromFile(defaultConfPath.get()); + conf.addPropsFromFile(defaultConfPath.get(), true); } return conf.getProps(); } @@ -141,11 +144,31 @@ public class DFSPropertiesConfiguration extends PropertiesConfig { } /** - * Add properties from external configuration files. + * Add properties from an external configuration file. A missing file is tolerated only when the + * caller's path equals {@link #DEFAULT_PATH} (the optional global {@code hudi-defaults.conf}); any + * other missing file fails fast with {@link HoodieIOException}, so a typo in an explicit + * user-supplied path (e.g. {@code --props /path/...}) is surfaced rather than silently loading + * empty properties. + * + * <p>For include-resolved paths use {@link #addPropsFromFile(StoragePath, boolean)} with + * {@code tolerateMissing=true}. * * @param filePath file path for configuration file. */ public void addPropsFromFile(StoragePath filePath) { + addPropsFromFile(filePath, filePath.equals(DEFAULT_PATH)); + } + + /** + * Add properties from an external configuration file. + * + * @param filePath file path for configuration file. + * @param tolerateMissing when {@code true}, a missing file is logged at {@code debug} and + * ignored (used for optional global-defaults paths and {@code include=} + * recursion). When {@code false}, missing files raise + * {@link HoodieIOException} so explicit user-supplied paths fail fast. + */ + void addPropsFromFile(StoragePath filePath, boolean tolerateMissing) { if (visitedFilePaths.contains(filePath.toString())) { throw new IllegalStateException("Loop detected; file " + filePath + " already referenced"); } @@ -156,9 +179,12 @@ public class DFSPropertiesConfiguration extends PropertiesConfig { ); try { - if (filePath.equals(DEFAULT_PATH) && !storage.exists(filePath)) { - log.debug("Properties file {} not found. Ignoring to load props file", filePath); - return; + if (!storage.exists(filePath)) { + if (tolerateMissing) { + log.debug("Properties file {} not found. Ignoring to load props file", filePath); + return; + } + throw new HoodieIOException("Properties file does not exist: " + filePath); } } catch (IOException ioe) { throw new HoodieIOException("Cannot check if the properties file exist: " + filePath, ioe); @@ -195,7 +221,9 @@ public class DFSPropertiesConfiguration extends PropertiesConfig { && cfgFilePath != null) { providedPath = new StoragePath(cfgFilePath.getParent(), split[1]); } - addPropsFromFile(providedPath); + // include= references may legitimately point to optional files (e.g. environment- + // specific overrides); skip silently when missing rather than failing the whole load. + addPropsFromFile(providedPath, true); } else { hoodieConfig.setValue(split[0], split[1]); } diff --git a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/util/TestDFSPropertiesConfiguration.java b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/util/TestDFSPropertiesConfiguration.java index 9e5e45a00778..fd2b55025985 100644 --- a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/util/TestDFSPropertiesConfiguration.java +++ b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/util/TestDFSPropertiesConfiguration.java @@ -271,4 +271,87 @@ public class TestDFSPropertiesConfiguration { TypedProperties props = cfg.getProps(); assertEquals(5, props.size()); } + + @Test + public void testIncludeNonExistentFile() throws IOException { + // Create a properties file that includes a non-existent file + Path filePath = new Path(dfsBasePath + "/t5.props"); + writePropertiesFile(filePath, new String[] { + "existing.prop=value1", + "include=" + dfsBasePath + "/non-existent-file.props", + "another.prop=value2" + }); + + // Should not throw an exception, but log a warning and continue + DFSPropertiesConfiguration cfg = new DFSPropertiesConfiguration(dfs.getConf(), + new StoragePath(filePath.toUri())); + TypedProperties props = cfg.getProps(); + + // Properties before and after the non-existent include should still be loaded + assertEquals(2, props.size()); + assertEquals("value1", props.getString("existing.prop")); + assertEquals("value2", props.getString("another.prop")); + } + + @Test + public void testIncludeNonExistentRelativeFile() throws IOException { + // Create a properties file that includes a non-existent relative file + Path filePath = new Path(dfsBasePath + "/t6.props"); + writePropertiesFile(filePath, new String[] { + "prop1=val1", + "include=non-existent-relative.props", + "prop2=val2" + }); + + // Should not throw an exception for non-existent relative includes + DFSPropertiesConfiguration cfg = new DFSPropertiesConfiguration(dfs.getConf(), + new StoragePath(filePath.toUri())); + TypedProperties props = cfg.getProps(); + + // Properties before and after the non-existent include should still be loaded + assertEquals(2, props.size()); + assertEquals("val1", props.getString("prop1")); + assertEquals("val2", props.getString("prop2")); + } + + @Test + public void testMixedExistentAndNonExistentIncludes() throws IOException { + // Create a properties file with both existent and non-existent includes + Path filePath = new Path(dfsBasePath + "/t7.props"); + writePropertiesFile(filePath, new String[] { + "base.prop=base_value", + "include=" + dfsBasePath + "/non-existent-1.props", + "include=" + dfsBasePath + "/t1.props", // This exists + "include=" + dfsBasePath + "/non-existent-2.props", + "override.prop=override_value" + }); + + // Should load successfully, ignoring non-existent files + DFSPropertiesConfiguration cfg = new DFSPropertiesConfiguration(dfs.getConf(), + new StoragePath(filePath.toUri())); + TypedProperties props = cfg.getProps(); + + // Should have properties from t1.props and the main file + assertEquals("base_value", props.getString("base.prop")); + assertEquals("override_value", props.getString("override.prop")); + assertEquals(123, props.getInteger("int.prop")); // From t1.props + assertEquals("str", props.getString("string.prop")); // From t1.props + assertTrue(props.getBoolean("boolean.prop")); // From t1.props + } + + @Test + public void testExplicitMissingPropertiesFileThrows() { + // Explicit user-supplied paths (e.g. --props /typo.props) should fail fast rather than + // silently load empty properties. Only include= recursion and optional global-defaults + // paths are tolerated. + StoragePath missingPath = new StoragePath(dfsBasePath + "/this-file-does-not-exist.props"); + assertThrows(HoodieIOException.class, + () -> new DFSPropertiesConfiguration(dfs.getConf(), missingPath), + "Constructor should throw when the explicit properties file is missing"); + + DFSPropertiesConfiguration cfg = new DFSPropertiesConfiguration(); + assertThrows(HoodieIOException.class, + () -> cfg.addPropsFromFile(missingPath), + "Public addPropsFromFile should throw when the explicit properties file is missing"); + } }
