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

[Eason09053360]

What changes were proposed in this pull request?

OzoneManagerRatisServer.newRaftProperties() applies two different config keys to the same
Ratis property, raft.server.retrycache.expirytime:

1. setRaftRetryCacheProperties() reads ozone.om.ratis.server.retry.cache.timeout
   (documented in ozone-default.xml with a 600000ms default) and calls
   RaftServerConfigKeys.RetryCache.setExpiryTime().
2. The last statement of newRaftProperties() is
   getOMHAConfigs(conf).forEach(properties::set), which copies every ozone.om.ha.*
   property into the RaftProperties with the prefix trimmed.
   OzoneManagerRatisServerConfig declares ozone.om.ha.raft.server.retrycache.expirytime
   with @Config(defaultValue = "300s"), which the config annotation processor emits into
   ozone-manager-default.xml. Since that is an OzoneConfiguration default resource, the
   key is always present even when the operator never sets it.

Step 2 runs after step 1, so ozone.om.ratis.server.retry.cache.timeout is unconditionally
overwritten and can never take effect. Operators tuning the documented key get no effect and
no warning, and the advertised 600000ms default has never been the effective value — the
effective default is 300s.

Introduced by HDDS-4329 (4669043), which added the new config and the getOMHAConfigs()
override without removing the old key.

This patch registers the old key as a deprecated alias of
ozone.om.ha.raft.server.retrycache.expirytime, so existing deployments that set it keep
working (and now actually take effect, with a deprecation warning), then removes the dead
setRaftRetryCacheProperties() path, the unused OMConfigKeys constants, and the stale
ozone-default.xml entry.

Compatibility note

No behaviour change for anyone who never set the old key: 300s was already the effective
value. For anyone who did set it, the configured value now takes effect instead of being
silently discarded. That is the point of the fix, but it is operator-visible.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16254

How was this patch tested?

New unit test TestOzoneManagerRatisServer#testRetryCacheExpiryTime asserts all three cases:
the 300s default, the current key being honoured, and the deprecated key now reaching Ratis
instead of being overwritten.

• TestOzoneManagerRatisServer — 7/7 pass
• TestOzoneConfiguration — 48/48 pass
• TestOzoneConfigurationFields — 5/5 pass (guards ozone-default.xml against the removed key)
• checkstyle.sh — 0 violations

Generated-by: Claude Code (Claude Opus 5)

[Copilot]

Pull request overview

This PR fixes a long-standing configuration override issue in OM’s Ratis retrycache.expirytime by deprecating the ineffective legacy key (ozone.om.ratis.server.retry.cache.timeout) and wiring it as an alias to the effective HA-prefixed key (ozone.om.ha.raft.server.retrycache.expirytime). This ensures operators who set the old key now get the intended behavior (with a deprecation warning), while removing dead code and stale configuration documentation.

Changes:

• Register ozone.om.ratis.server.retry.cache.timeout as a deprecated alias of ozone.om.ha.raft.server.retrycache.expirytime.
• Remove the unused OM-side retry cache expiry setter and the corresponding unused OMConfigKeys constants + ozone-default.xml entry.
• Add a unit test verifying the effective default and that both the current and deprecated keys are honored.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java	Adds unit test covering default, current key, and deprecated key behavior for retry cache expiry.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java	Removes dead retry-cache configuration path so the HA-prefixed Ratis config mapping remains the single source of truth.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java	Removes unused legacy config key constants for the deprecated setting.
hadoop-hdds/common/src/main/resources/ozone-default.xml	Drops stale documentation entry for the ineffective legacy key.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java	Adds deprecation mapping so the legacy key is translated to the effective HA-prefixed key.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chihsuan]

No behaviour change for anyone who never set the old key: 300s was already the effective value. For anyone who did set it, the configured value now takes effect instead of being silently discarded. That is the point of the fix, but it is operator-visible.

Thanks for the patch! @Eason09053360 Quick question on the migration path. The old code seems read this key as milliseconds, but through the alias, Ratis falls back to its own unit, so I wasn't sure what happens to a cluster that set a bare 600000. Would that end up much longer than intended? And do operators actually get a deprecation warning here, given how the value is read?

[Eason09053360]

Thanks for the patch! @Eason09053360 Quick question on the migration path. The old code seems read this key as milliseconds, but through the alias, Ratis falls back to its own unit, so I wasn't sure what happens to a cluster that set a bare 600000. Would that end up much longer than intended? And do operators actually get a deprecation warning here, given how the value is read?

Thanks @chihsuan, you're right on both counts.

Units. The old path read the key with TimeUnit.MILLISECONDS as the fallback unit, so a bare 600000 meant 10 minutes. Through the alias the raw string goes straight to Ratis, which falls back to seconds (RetryCache.EXPIRY_TIME_DEFAULT = 60s) — so 600000 becomes ~6.9 days. 600000ms and 300s are unaffected. My test used 17s, which carries a unit, so it missed this.

About Deprecation warning. There is none. Hadoop logs it only on get()/set() of the old key, and loading it from XML just copies the value across silently. This patch removed the only get(), so nothing fires.

I dropped the alias and restored setRaftRetryCacheProperties() instead, now called below getOMHAConfigs(conf).forEach(properties::set) — that reverses the override behind the original bug — and applied only when the old key is set. Millisecond semantics are preserved and the deprecation warning is logged explicitly.

The alias could not stay: with it registered, conf.get(oldKey) resolves to the current key, which always carries the generated 300s default. It is never null, so there is no way to tell whether the operator set anything, and every OM start would log a spurious deprecation warning. Removing the ozone-default.xml entry is now load-bearing for that null check.

The test covers a bare 600000, both keys set (the deprecated one wins, being applied last), and asserts the warning is logged.