This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 5ec55033ae [#13992] fix(fileset): Keep bypass-prefixed HDFS settings
over the injected defaults (#13996)
5ec55033ae is described below
commit 5ec55033aee2ccec4c95d70357d553d890fde288
Author: YangJie <[email protected]>
AuthorDate: Tue Oct 27 04:19:42 2026 -0400
[#13992] fix(fileset): Keep bypass-prefixed HDFS settings over the injected
defaults (#13996)
### What changes were proposed in this pull request?
- `HDFSFileSystemProvider.additionalHDFSConfig` no longer injects the
`ipc.client.connect.timeout` and `ipc.client.ping` defaults when the
user set the key with the `gravitino.bypass.` prefix.
- `FileSystemUtils.createConfiguration` writes the bypass keys before
the plain keys, so a plain key always wins over its bypass twin, as
`toHadoopConfigMap` documents. The prefix is now only stripped from the
start of a key.
### Why are the changes needed?
The injected default and the user's bypass key both reached
`createConfiguration`, which wrote them in `HashMap` order into one
Hadoop resource. Which value won depended on how many properties the map
held, so the user's setting was silently ignored once the catalog had
enough properties.
Fix: #13992
### Does this PR introduce _any_ user-facing change?
A `gravitino.bypass.` setting for the IPC keys now always overrides the
defaults. When the same key is set both plainly and with the bypass
prefix, for example at different levels of catalog, schema and fileset,
the plain value now always applies; before, either one could apply
depending on the number of properties.
### How was this patch tested?
`TestFileSystemUtils.testBypassValueOverridesInjectedHdfsDefault` and
`testPlainKeyWinsOverBypassTwin` build the configuration with 0 to 200
unrelated properties and check the expected value at every size.
---------
Co-authored-by: Jerry Shao <[email protected]>
---
.../catalog/fileset/fs/TestFileSystemUtils.java | 41 ++++++++++++++++++++++
.../catalog/hadoop/fs/FileSystemUtils.java | 11 ++++--
.../catalog/hadoop/fs/HDFSFileSystemProvider.java | 9 +++--
3 files changed, 55 insertions(+), 6 deletions(-)
diff --git
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/fs/TestFileSystemUtils.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/fs/TestFileSystemUtils.java
index 1dbc80d8b4..27d4b625fb 100644
---
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/fs/TestFileSystemUtils.java
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/fs/TestFileSystemUtils.java
@@ -20,9 +20,12 @@
package org.apache.gravitino.catalog.fileset.fs;
import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
import java.util.Map;
import java.util.stream.Stream;
+import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemUtils;
+import org.apache.gravitino.catalog.hadoop.fs.HDFSFileSystemProvider;
import org.apache.hadoop.conf.Configuration;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -106,4 +109,42 @@ public class TestFileSystemUtils {
ImmutableMap.of("fs.s3a.endpoint", "v1", "fs.s3a.impl", "v2"),
ImmutableMap.of("s3a-endpoint", "fs.s3a.endpoint")));
}
+
+ @Test
+ void testBypassValueOverridesInjectedHdfsDefault() {
+ // The ipc timeout default is injected for HDFS; a user value set with the
bypass prefix must
+ // win however many other properties the map holds.
+ for (int fillers = 0; fillers <= 200; fillers++) {
+ Map<String, String> config = fillerProperties(fillers);
+ config.put(FileSystemProvider.GRAVITINO_BYPASS +
"ipc.client.connect.timeout", "60000");
+ Configuration configuration =
+ FileSystemUtils.createCompatibleConfiguration(
+ FileSystemProvider.GRAVITINO_BYPASS,
+ HDFSFileSystemProvider.additionalHDFSConfig(config));
+ Assertions.assertEquals(
+ "60000", configuration.get("ipc.client.connect.timeout"), "fillers:
" + fillers);
+ }
+ }
+
+ @Test
+ void testPlainKeyWinsOverBypassTwin() {
+ for (int fillers = 0; fillers <= 200; fillers++) {
+ Map<String, String> config = fillerProperties(fillers);
+ config.put("ipc.client.connect.timeout", "plain");
+ config.put(FileSystemProvider.GRAVITINO_BYPASS +
"ipc.client.connect.timeout", "bypass");
+ Configuration configuration =
+ FileSystemUtils.createCompatibleConfiguration(
+ FileSystemProvider.GRAVITINO_BYPASS, config);
+ Assertions.assertEquals(
+ "plain", configuration.get("ipc.client.connect.timeout"), "fillers:
" + fillers);
+ }
+ }
+
+ private static Map<String, String> fillerProperties(int count) {
+ Map<String, String> config = new HashMap<>();
+ for (int i = 0; i < count; i++) {
+ config.put("filler.key." + i, "v" + i);
+ }
+ return config;
+ }
}
diff --git
a/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java
b/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java
index 10265761d5..54f49412e9 100644
---
a/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java
+++
b/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java
@@ -211,9 +211,14 @@ public class FileSystemUtils {
writer.writeStartDocument();
writer.writeStartElement(CONFIG_ROOT);
- config.forEach(
- (k, v) ->
- writeProperty(writer, StringUtils.isNotBlank(bypass) ?
k.replace(bypass, "") : k, v));
+ // Configuration keeps the last value of a key within one resource.
Write the bypass keys
+ // first so that a plain key wins over its bypass twin, as in
toHadoopConfigMap.
+ config.entrySet().stream()
+ .filter(e -> StringUtils.isNotBlank(bypass) &&
e.getKey().startsWith(bypass))
+ .forEach(e -> writeProperty(writer,
e.getKey().substring(bypass.length()), e.getValue()));
+ config.entrySet().stream()
+ .filter(e -> StringUtils.isBlank(bypass) ||
!e.getKey().startsWith(bypass))
+ .forEach(e -> writeProperty(writer, e.getKey(), e.getValue()));
writer.writeEndElement();
writer.writeEndDocument();
writer.close();
diff --git
a/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/HDFSFileSystemProvider.java
b/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/HDFSFileSystemProvider.java
index 0711b008be..9296c0a6ef 100644
---
a/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/HDFSFileSystemProvider.java
+++
b/catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/HDFSFileSystemProvider.java
@@ -84,14 +84,17 @@ public class HDFSFileSystemProvider implements
FileSystemProvider {
public static Map<String, String> additionalHDFSConfig(Map<String, String>
configs) {
Map<String, String> additionalConfigs = Maps.newHashMap(configs);
- // Avoid multiple retries to speed up failure in test cases.
+ // Avoid multiple retries to speed up failure in test cases. A user value,
plain or with the
+ // bypass prefix, takes precedence over these defaults.
// Use hard code instead of
CommonConfigurationKeysPublic.IPC_CLIENT_CONNECT_TIMEOUT_KEY to
// avoid dependency on a specific Hadoop version.
- if (!configs.containsKey(HDFS_IPC_CLIENT_CONNECT_TIMEOUT_KEY)) {
+ if (!configs.containsKey(HDFS_IPC_CLIENT_CONNECT_TIMEOUT_KEY)
+ && !configs.containsKey(GRAVITINO_BYPASS +
HDFS_IPC_CLIENT_CONNECT_TIMEOUT_KEY)) {
additionalConfigs.put(HDFS_IPC_CLIENT_CONNECT_TIMEOUT_KEY,
DEFAULT_CONNECTION_TIMEOUT);
}
- if (!configs.containsKey(HDFS_IPC_PING_KEY)) {
+ if (!configs.containsKey(HDFS_IPC_PING_KEY)
+ && !configs.containsKey(GRAVITINO_BYPASS + HDFS_IPC_PING_KEY)) {
additionalConfigs.put(HDFS_IPC_PING_KEY, DEFAULT_HDFS_IPC_PING);
}
