From 81c1aa877cfd2ff5fbfe22ee2b2155afbd637828 Mon Sep 17 00:00:00 2001 From: Eason09053360 Date: Mon, 24 Aug 2026 09:53:06 +0800 Subject: [PATCH 1/2] HDDS-16254. Deprecate ineffective ozone.om.ratis.server.retry.cache.timeout newRaftProperties() set raft.server.retrycache.expirytime twice: once from ozone.om.ratis.server.retry.cache.timeout, then again from the ozone.om.ha.* copy at the end of the method, which always won. The old key never took effect. Map it to ozone.om.ha.raft.server.retrycache.expirytime as a deprecated key so existing configs keep working, and drop the dead code path and its stale ozone-default.xml entry. --- .../hadoop/hdds/conf/OzoneConfiguration.java | 2 ++ .../src/main/resources/ozone-default.xml | 7 ------- .../apache/hadoop/ozone/om/OMConfigKeys.java | 6 ------ .../om/ratis/OzoneManagerRatisServer.java | 12 ----------- .../om/ratis/TestOzoneManagerRatisServer.java | 21 +++++++++++++++++++ 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java index 69c4c029ce10..b4ee6fd7d426 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java @@ -318,6 +318,8 @@ private static void addDeprecatedKeys() { + RaftServerConfigKeys.PREFIX + "." + "rpcslowness.timeout", HDDS_DATANODE_RATIS_PREFIX_KEY + "." + RaftServerConfigKeys.PREFIX + "." + "rpc.slowness.timeout"), + new DeprecationDelta("ozone.om.ratis.server.retry.cache.timeout", + "ozone.om.ha.raft.server.retrycache.expirytime"), new DeprecationDelta("dfs.datanode.keytab.file", HddsConfigKeys.HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY), new DeprecationDelta("ozone.scm.chunk.layout", diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 9528ca27fc21..f277c320add4 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -2346,13 +2346,6 @@ The timeout duration for OM's ratis server request . - - ozone.om.ratis.server.retry.cache.timeout - 600000ms - OZONE, OM, RATIS, MANAGEMENT - Retry Cache entry timeout for OM's ratis server. - - ozone.om.ratis.minimum.timeout 5s diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 2990410fbaec..d5104ba212da 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -285,12 +285,6 @@ public final class OMConfigKeys { public static final TimeDuration OZONE_OM_RATIS_SERVER_REQUEST_TIMEOUT_DEFAULT = TimeDuration.valueOf(3000, TimeUnit.MILLISECONDS); - public static final String - OZONE_OM_RATIS_SERVER_RETRY_CACHE_TIMEOUT_KEY - = "ozone.om.ratis.server.retry.cache.timeout"; - public static final TimeDuration - OZONE_OM_RATIS_SERVER_RETRY_CACHE_TIMEOUT_DEFAULT - = TimeDuration.valueOf(600000, TimeUnit.MILLISECONDS); public static final String OZONE_OM_RATIS_MINIMUM_TIMEOUT_KEY = "ozone.om.ratis.minimum.timeout"; public static final TimeDuration OZONE_OM_RATIS_MINIMUM_TIMEOUT_DEFAULT diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java index 18defcf808a2..de4a08c25093 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java @@ -752,8 +752,6 @@ public static RaftProperties newRaftProperties(ConfigurationSource conf, setRaftRpcProperties(properties, conf); - setRaftRetryCacheProperties(properties, conf); - setRaftSnapshotProperties(properties, conf); setRaftCloseThreshold(properties, conf); @@ -846,16 +844,6 @@ private static void setRaftRpcProperties(RaftProperties properties, Configuratio RaftServerConfigKeys.Rpc.setSlownessTimeout(properties, nodeFailureTimeout); } - private static void setRaftRetryCacheProperties(RaftProperties properties, ConfigurationSource conf) { - // Set timeout for server retry cache entry - TimeUnit retryCacheTimeoutUnit = OMConfigKeys.OZONE_OM_RATIS_SERVER_RETRY_CACHE_TIMEOUT_DEFAULT.getUnit(); - final TimeDuration retryCacheTimeout = TimeDuration.valueOf(conf.getTimeDuration( - OMConfigKeys.OZONE_OM_RATIS_SERVER_RETRY_CACHE_TIMEOUT_KEY, - OMConfigKeys.OZONE_OM_RATIS_SERVER_RETRY_CACHE_TIMEOUT_DEFAULT.getDuration(), retryCacheTimeoutUnit), - retryCacheTimeoutUnit); - RaftServerConfigKeys.RetryCache.setExpiryTime(properties, retryCacheTimeout); - } - private static void setRaftSnapshotProperties(RaftProperties properties, ConfigurationSource conf) { // Set auto trigger snapshot. We don't need to configure auto trigger // threshold in OM, as last applied index is flushed during double buffer diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java index 6eda7a6ba1ea..dc100ffcb613 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java @@ -49,7 +49,9 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.security.OMCertificateClient; import org.apache.ozone.test.GenericTestUtils.LogCapturer; +import org.apache.ratis.conf.RaftProperties; import org.apache.ratis.protocol.RaftGroupId; +import org.apache.ratis.server.RaftServerConfigKeys; import org.apache.ratis.server.protocol.TermIndex; import org.apache.ratis.statemachine.SnapshotInfo; import org.apache.ratis.util.ExitUtils; @@ -275,4 +277,23 @@ public void verifyRaftGroupIdGenerationWithCustomOmServiceId(@TempDir Path metaD assertEquals(raftGroupId.toByteString().size(), 16); newOmRatisServer.stop(); } + + @Test + public void testRetryCacheExpiryTime(@TempDir Path ratisDir) { + assertEquals(300_000, retryCacheExpiryMillis(new OzoneConfiguration(), ratisDir)); + + OzoneConfiguration currentKeyConf = new OzoneConfiguration(); + currentKeyConf.set(OMConfigKeys.OZONE_OM_HA_PREFIX + ".raft.server.retrycache.expirytime", "42s"); + assertEquals(42_000, retryCacheExpiryMillis(currentKeyConf, ratisDir)); + + // The deprecated key must reach Ratis instead of being silently overwritten. + OzoneConfiguration deprecatedKeyConf = new OzoneConfiguration(); + deprecatedKeyConf.set("ozone.om.ratis.server.retry.cache.timeout", "17s"); + assertEquals(17_000, retryCacheExpiryMillis(deprecatedKeyConf, ratisDir)); + } + + private static long retryCacheExpiryMillis(OzoneConfiguration conf, Path ratisDir) { + RaftProperties properties = OzoneManagerRatisServer.newRaftProperties(conf, 9872, ratisDir.toString()); + return RaftServerConfigKeys.RetryCache.expiryTime(properties).toLong(TimeUnit.MILLISECONDS); + } } From 4cd875adfbb557e8dc1fb3ff17789e727b5711ac Mon Sep 17 00:00:00 2001 From: Eason09053360 Date: Wed, 26 Aug 2026 01:30:12 +0800 Subject: [PATCH 2/2] HDDS-16254. Honour the deprecated key directly instead of aliasing it --- .../hadoop/hdds/conf/OzoneConfiguration.java | 2 -- .../om/ratis/OzoneManagerRatisServer.java | 18 +++++++++++++++ .../om/ratis/TestOzoneManagerRatisServer.java | 23 ++++++++++++++++--- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java index b4ee6fd7d426..69c4c029ce10 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java @@ -318,8 +318,6 @@ private static void addDeprecatedKeys() { + RaftServerConfigKeys.PREFIX + "." + "rpcslowness.timeout", HDDS_DATANODE_RATIS_PREFIX_KEY + "." + RaftServerConfigKeys.PREFIX + "." + "rpc.slowness.timeout"), - new DeprecationDelta("ozone.om.ratis.server.retry.cache.timeout", - "ozone.om.ha.raft.server.retrycache.expirytime"), new DeprecationDelta("dfs.datanode.keytab.file", HddsConfigKeys.HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY), new DeprecationDelta("ozone.scm.chunk.layout", diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java index de4a08c25093..3e88e269e200 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java @@ -115,6 +115,9 @@ public final class OzoneManagerRatisServer { private static final Logger LOG = LoggerFactory.getLogger(OzoneManagerRatisServer.class); + /** Superseded by {@code ozone.om.ha.raft.server.retrycache.expirytime}, still honoured if set. */ + private static final String RETRY_CACHE_TIMEOUT_DEPRECATED_KEY = "ozone.om.ratis.server.retry.cache.timeout"; + private final int port; private final RaftServer server; private final Supplier serverDivision; @@ -757,6 +760,10 @@ public static RaftProperties newRaftProperties(ConfigurationSource conf, setRaftCloseThreshold(properties, conf); getOMHAConfigs(conf).forEach(properties::set); + + // Must run after the ozone.om.ha.* copy above, which would otherwise override it. + setRaftRetryCacheProperties(properties, conf); + return properties; } @@ -844,6 +851,17 @@ private static void setRaftRpcProperties(RaftProperties properties, Configuratio RaftServerConfigKeys.Rpc.setSlownessTimeout(properties, nodeFailureTimeout); } + private static void setRaftRetryCacheProperties(RaftProperties properties, ConfigurationSource conf) { + if (conf.get(RETRY_CACHE_TIMEOUT_DEPRECATED_KEY) == null) { + return; + } + final String currentKey = OZONE_OM_HA_PREFIX + "." + RaftServerConfigKeys.RetryCache.EXPIRY_TIME_KEY; + LOG.warn("{} is deprecated. Instead, use {}.", RETRY_CACHE_TIMEOUT_DEPRECATED_KEY, currentKey); + // A value without a unit suffix is read as milliseconds, as the deprecated key always has been. + final long timeout = conf.getTimeDuration(RETRY_CACHE_TIMEOUT_DEPRECATED_KEY, 0, TimeUnit.MILLISECONDS); + RaftServerConfigKeys.RetryCache.setExpiryTime(properties, TimeDuration.valueOf(timeout, TimeUnit.MILLISECONDS)); + } + private static void setRaftSnapshotProperties(RaftProperties properties, ConfigurationSource conf) { // Set auto trigger snapshot. We don't need to configure auto trigger // threshold in OM, as last applied index is flushed during double buffer diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java index dc100ffcb613..326a2cc18381 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java @@ -73,6 +73,9 @@ public class TestOzoneManagerRatisServer { private OzoneManagerRatisServer omRatisServer; private String clientId = UUID.randomUUID().toString(); private static final long RATIS_RPC_TIMEOUT = 500L; + private static final String CURRENT_RETRY_CACHE_KEY = + OMConfigKeys.OZONE_OM_HA_PREFIX + "." + RaftServerConfigKeys.RetryCache.EXPIRY_TIME_KEY; + private static final String DEPRECATED_RETRY_CACHE_KEY = "ozone.om.ratis.server.retry.cache.timeout"; private OMMetadataManager omMetadataManager; private OzoneManager ozoneManager; private OMNodeDetails omNodeDetails; @@ -283,13 +286,27 @@ public void testRetryCacheExpiryTime(@TempDir Path ratisDir) { assertEquals(300_000, retryCacheExpiryMillis(new OzoneConfiguration(), ratisDir)); OzoneConfiguration currentKeyConf = new OzoneConfiguration(); - currentKeyConf.set(OMConfigKeys.OZONE_OM_HA_PREFIX + ".raft.server.retrycache.expirytime", "42s"); + currentKeyConf.set(CURRENT_RETRY_CACHE_KEY, "42s"); assertEquals(42_000, retryCacheExpiryMillis(currentKeyConf, ratisDir)); - // The deprecated key must reach Ratis instead of being silently overwritten. + // The deprecated key must reach Ratis instead of being silently overwritten, and must warn. + LogCapturer logCapturer = LogCapturer.captureLogs(OzoneManagerRatisServer.class); OzoneConfiguration deprecatedKeyConf = new OzoneConfiguration(); - deprecatedKeyConf.set("ozone.om.ratis.server.retry.cache.timeout", "17s"); + deprecatedKeyConf.set(DEPRECATED_RETRY_CACHE_KEY, "17s"); assertEquals(17_000, retryCacheExpiryMillis(deprecatedKeyConf, ratisDir)); + assertThat(logCapturer.getOutput()).contains(DEPRECATED_RETRY_CACHE_KEY + " is deprecated"); + + // A value without a unit suffix keeps the milliseconds the deprecated key was always read with, + // instead of falling back to the seconds Ratis would assume. + OzoneConfiguration bareValueConf = new OzoneConfiguration(); + bareValueConf.set(DEPRECATED_RETRY_CACHE_KEY, "600000"); + assertEquals(600_000, retryCacheExpiryMillis(bareValueConf, ratisDir)); + + // The deprecated key is applied last, so it wins when both are set. + OzoneConfiguration bothKeysConf = new OzoneConfiguration(); + bothKeysConf.set(CURRENT_RETRY_CACHE_KEY, "42s"); + bothKeysConf.set(DEPRECATED_RETRY_CACHE_KEY, "17s"); + assertEquals(17_000, retryCacheExpiryMillis(bothKeysConf, ratisDir)); } private static long retryCacheExpiryMillis(OzoneConfiguration conf, Path ratisDir) {