This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new c6f51fbec4c HDDS-16254. Deprecate ineffective
ozone.om.ratis.server.retry.cache.timeout (#11096)
c6f51fbec4c is described below
commit c6f51fbec4cfbaeb3f137eb1a5d5bb17e0999928
Author: Y-C <[email protected]>
AuthorDate: Mon Sep 21 16:27:14 2026 +0800
HDDS-16254. Deprecate ineffective ozone.om.ratis.server.retry.cache.timeout
(#11096)
---
.../common/src/main/resources/ozone-default.xml | 7 ---
.../hadoop/hdds/conf/ConfigurationSource.java | 8 ++++
.../hdds/conf/MutableConfigurationSource.java | 8 ----
.../org/apache/hadoop/ozone/om/OMConfigKeys.java | 6 ---
.../ozone/om/ratis/OzoneManagerRatisServer.java | 30 +++++++++----
.../om/ratis/TestOzoneManagerRatisServer.java | 50 ++++++++++++++++++++++
6 files changed, 79 insertions(+), 30 deletions(-)
diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml
b/hadoop-hdds/common/src/main/resources/ozone-default.xml
index 2de7451e1d2..51e66a3618c 100644
--- a/hadoop-hdds/common/src/main/resources/ozone-default.xml
+++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml
@@ -2394,13 +2394,6 @@
<description>The timeout duration for OM's ratis server request
.</description>
</property>
- <property>
- <name>ozone.om.ratis.server.retry.cache.timeout</name>
- <value>600000ms</value>
- <tag>OZONE, OM, RATIS, MANAGEMENT</tag>
- <description>Retry Cache entry timeout for OM's ratis server.</description>
- </property>
-
<property>
<name>ozone.om.ratis.minimum.timeout</name>
<value>5s</value>
diff --git
a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
index 4b9dae7265b..247d183276b 100644
---
a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
+++
b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
@@ -156,6 +156,14 @@ default boolean isConfigured(String key) {
return get(key) != null;
}
+ /**
+ * Returns whether a property has an explicitly configured value.
+ * By default, any non-null value is explicit; implementations may
distinguish default resources.
+ */
+ default boolean isExplicitlySet(String key) {
+ return get(key) != null;
+ }
+
/**
* Create a Configuration object and inject the required configuration
values.
*
diff --git
a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/MutableConfigurationSource.java
b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/MutableConfigurationSource.java
index 452edc98557..11f1cffe0a2 100644
---
a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/MutableConfigurationSource.java
+++
b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/MutableConfigurationSource.java
@@ -26,14 +26,6 @@
public interface MutableConfigurationSource
extends ConfigurationSource, ConfigurationTarget {
- /**
- * Returns whether a property has an explicitly configured value.
- * By default, any non-null value is explicit; implementations may
distinguish default resources.
- */
- default boolean isExplicitlySet(String key) {
- return get(key) != null;
- }
-
/**
* Sets {@code value} for {@code key} only if the key is not already set.
* Default implementation treats any non-null {@link #get(String)} result as
set.
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 5852e06ea75..83d2507accd 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 18defcf808a..919ad193bb9 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<RaftServer.Division> serverDivision;
@@ -752,13 +755,15 @@ public static RaftProperties
newRaftProperties(ConfigurationSource conf,
setRaftRpcProperties(properties, conf);
- setRaftRetryCacheProperties(properties, conf);
-
setRaftSnapshotProperties(properties, 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;
}
@@ -847,13 +852,20 @@ private static void setRaftRpcProperties(RaftProperties
properties, Configuratio
}
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);
+ if (conf.get(RETRY_CACHE_TIMEOUT_DEPRECATED_KEY) == null) {
+ return;
+ }
+ final String currentKey = OZONE_OM_HA_PREFIX + "." +
RaftServerConfigKeys.RetryCache.EXPIRY_TIME_KEY;
+ if (conf.isExplicitlySet(currentKey)) {
+ LOG.warn("{} is deprecated and ignored, because {} is set.",
RETRY_CACHE_TIMEOUT_DEPRECATED_KEY, currentKey);
+ return;
+ }
+ // A value without a unit suffix is read as milliseconds, as the
deprecated key always has been.
+ final TimeDuration timeout = TimeDuration.valueOf(
+ conf.getTimeDuration(RETRY_CACHE_TIMEOUT_DEPRECATED_KEY, 0,
TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
+ // Spell out the resolved value, so it can be copied to the current key
without changing meaning.
+ LOG.warn("{} is deprecated. Instead, use {} = {}.",
RETRY_CACHE_TIMEOUT_DEPRECATED_KEY, currentKey, timeout);
+ RaftServerConfigKeys.RetryCache.setExpiryTime(properties, timeout);
}
private static void setRaftSnapshotProperties(RaftProperties properties,
ConfigurationSource conf) {
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 6eda7a6ba1e..da4212822d0 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,11 +49,14 @@
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;
import org.apache.ratis.util.LifeCycle;
+import org.apache.ratis.util.TimeDuration;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
@@ -71,6 +74,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;
@@ -275,4 +281,48 @@ 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(CURRENT_RETRY_CACHE_KEY, "42s");
+ assertEquals(42_000, retryCacheExpiryMillis(currentKeyConf, ratisDir));
+
+ // 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(DEPRECATED_RETRY_CACHE_KEY, "17s");
+ assertEquals(17_000, retryCacheExpiryMillis(deprecatedKeyConf, ratisDir));
+ assertThat(logCapturer.getOutput()).contains(deprecationWarning(17_000));
+
+ // 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. The warning
must name that resolved
+ // value, so copying it to the current key does not silently reinterpret
it as seconds.
+ logCapturer.clearOutput();
+ OzoneConfiguration bareValueConf = new OzoneConfiguration();
+ bareValueConf.set(DEPRECATED_RETRY_CACHE_KEY, "600000");
+ assertEquals(600_000, retryCacheExpiryMillis(bareValueConf, ratisDir));
+ assertThat(logCapturer.getOutput()).contains(deprecationWarning(600_000));
+
+ // An explicitly set current key wins over the deprecated one, which is
then reported as ignored.
+ logCapturer.clearOutput();
+ OzoneConfiguration bothKeysConf = new OzoneConfiguration();
+ bothKeysConf.set(CURRENT_RETRY_CACHE_KEY, "42s");
+ bothKeysConf.set(DEPRECATED_RETRY_CACHE_KEY, "17s");
+ assertEquals(42_000, retryCacheExpiryMillis(bothKeysConf, ratisDir));
+ assertThat(logCapturer.getOutput()).contains(DEPRECATED_RETRY_CACHE_KEY +
" is deprecated and ignored, because "
+ + CURRENT_RETRY_CACHE_KEY + " is set.");
+ }
+
+ private static String deprecationWarning(long expectedMillis) {
+ return DEPRECATED_RETRY_CACHE_KEY + " is deprecated. Instead, use " +
CURRENT_RETRY_CACHE_KEY + " = "
+ + TimeDuration.valueOf(expectedMillis, TimeUnit.MILLISECONDS) + ".";
+ }
+
+ private static long retryCacheExpiryMillis(OzoneConfiguration conf, Path
ratisDir) {
+ RaftProperties properties =
OzoneManagerRatisServer.newRaftProperties(conf, 9872, ratisDir.toString());
+ return
RaftServerConfigKeys.RetryCache.expiryTime(properties).toLong(TimeUnit.MILLISECONDS);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
