HDDS-3964. Ratis config key mismatch

[adoroszlai]

What changes were proposed in this pull request?

1. Fix config key mismatch: raft.server.notification.no-leader.timeout vs. raft.server.Notification.no-leader.timeout.
2. Use type-safe config for Raft client settings in tests. This ensures that we use the correct key (raft.client.rpc.watch.request.timeout) instead of similar but wrong one (raft.client.watch.request.timeout). Get rid of constants to discourage setting raw config.
3. Fix prefix for RatisClientConfig.RaftConfig. @ConfigGroup defined in inner class (RaftConfig) does not inherit prefix (hdds.ratis) from outer class (RatisClientConfig), so we need to explicitly specify it. Without the patch we got:

   $ grep -A1 raft.client hadoop-hdds/common/target/classes/ozone-default-generated.xml
       <name>raft.client.async.outstanding-requests.max</name>
       <value>32</value>
   --
       <name>raft.client.rpc.request.timeout</name>
       <value>60s</value>
   --
       <name>raft.client.rpc.watch.request.timeout</name>
       <value>180s</value>
   

   so RatisHelper would not apply these settings to RaftProperties.

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

How was this patch tested?

Verified ozone-default-generated.xml has hdds.ratis prefix:

$ grep -A1 raft.client hadoop-hdds/common/target/classes/ozone-default-generated.xml
    <name>hdds.ratis.raft.client.async.outstanding-requests.max</name>
    <value>32</value>
--
    <name>hdds.ratis.raft.client.rpc.request.timeout</name>
    <value>60s</value>
--
    <name>hdds.ratis.raft.client.rpc.watch.request.timeout</name>
    <value>180s</value>

Ran some affected integration tests locally, verified that custom Ratis settings were applied:

[pool-63-thread-1] INFO  server.RaftServerConfigKeys (ConfUtils.java:logGet(44)) - raft.server.notification.no-leader.timeout = 10000ms (custom)

(client property printed by temporary log statement):

[Command processor thread] INFO  ratis.RatisHelper (RatisHelper.java:createRaftClientProperties(241)) - ZZZ raft.client.rpc.watch.request.timeout = 3000ms

https://github.com/adoroszlai/hadoop-ozone/runs/873847680

[adoroszlai]

@cku328 thanks for finding the watch.request.timeout problem, please review

[cku328]

Thanks @adoroszlai for fixing this. Overall LGTM.

Would it be better to change clientConfig to ratisClientConfig so that it matches ratisServerConfig (#1106)?

[adoroszlai]

Would it be better to change clientConfig to ratisClientConfig so that it matches ratisServerConfig (#1106)?

Thanks @cku328 for the suggestion. I'm OK with both names. I'll update the patch based on your suggestion together with any other changes that may be requested by @bshashikant and/or @lokeshj1703, or if they approve it as it is. (I'd like to minimize number of CI checks due to too much flakiness.)

[lokeshj1703]

@adoroszlai Thanks for working on this! The changes look good to me. +1.
Minor comment - We can add suffix like "InMs" for time based configs in classes like RatisClientConfig.
We can do it as part of separate jira though.

[adoroszlai]

Thanks @lokeshj1703 for the review.

Minor comment - We can add suffix like "InMs" for time based configs in classes like RatisClientConfig.
We can do it as part of separate jira though.

I was considering changing the parameter to Duration, similar to

https://github.com/apache/hadoop-ozone/blob/de027855798bf3b891b8d3c00dc8e59531f98781/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java#L41-L49

What do you think about that?

[lokeshj1703]

I was considering changing the parameter to Duration, similar to

https://github.com/apache/hadoop-ozone/blob/de027855798bf3b891b8d3c00dc8e59531f98781/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java#L41-L49

What do you think about that?

yeah...that seems like a better approach.

[adoroszlai]

I was considering changing the parameter to Duration
yeah...that seems like a better approach.

Created HDDS-3975 for this.

[adoroszlai]

Thanks @cku328 and @lokeshj1703 for the review.