HDDS-16254. Deprecate ineffective ozone.om.ratis.server.retry.cache.timeout

hadoop-hdds/common/src/main/resources/ozone-default.xml

@@ -2346,13 +2346,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>

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

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,14 @@ 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;
+    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) {

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;
@@ -71,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;
@@ -275,4 +280,37 @@ 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(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) {
+    RaftProperties properties = OzoneManagerRatisServer.newRaftProperties(conf, 9872, ratisDir.toString());
+    return RaftServerConfigKeys.RetryCache.expiryTime(properties).toLong(TimeUnit.MILLISECONDS);
+  }
 }