-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-14030. Add ConfigGroup prefix to all configs where missing #9460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Russole for working on this patch.
I have checked all the places configKey include configGroup(Prefix).
...p-hdds/framework/src/main/java/org/apache/hadoop/hdds/fs/DedicatedDiskSpaceUsageFactory.java
Show resolved
Hide resolved
adoroszlai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Russole for the patch.
| public static final String OM_CLIENT_RPC_TIME_OUT = "rpc.timeout"; | ||
| public static final String OM_CLIENT_RPC_TIME_OUT = "ozone.om.client.rpc.timeout"; | ||
| public static final String OM_TRASH_EMPTIER_CORE_POOL_SIZE | ||
| = "trash.core.pool.size"; | ||
| = "ozone.om.client.trash.core.pool.size"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please inline these constants (use string value directly in key). They are not used elsewhere.
| public static final String SCM_CLIENT_RPC_TIME_OUT = "rpc.timeout"; | ||
| public static final String SCM_CLIENT_RPC_TIME_OUT = "hdds.scmclient.rpc.timeout"; | ||
| public static final String SCM_CLIENT_FAILOVER_MAX_RETRY = | ||
| "failover.max.retry"; | ||
| "hdds.scmclient.failover.max.retry"; | ||
| public static final String SCM_CLIENT_MAX_RETRY_TIMEOUT = | ||
| "max.retry.timeout"; | ||
| "hdds.scmclient.max.retry.timeout"; | ||
| public static final String SCM_CLIENT_RETRY_INTERVAL = | ||
| "failover.retry.interval"; | ||
| "hdds.scmclient.failover.retry.interval"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please inline these constants.
| public static final String PREFIX = "hdds.datanode.replication"; | ||
| public static final String STREAMS_LIMIT_KEY = "streams.limit"; | ||
| public static final String QUEUE_LIMIT = "queue.limit"; | ||
| public static final String QUEUE_LIMIT = "hdds.datanode.replication.queue.limit"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please inline this constant.
|
Thank you @Gargi-jais11 and @adoroszlai for the helpful review comments. |
Gargi-jais11
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Thanks @Russole for the patch, @Gargi-jais11 for the review. |
What changes were proposed in this pull request?
This PR standardizes configuration key definitions across the codebase.
Many configuration classes annotated with
@ConfigGroup(prefix = "...")still use unprefixed keys in their@Config(key = ...)entries (e.g., "shutdown.timeout" instead of "ozone.service.shutdown.timeout").This patch updates those entries to explicitly include the full prefix in the key literal.
Benefits of this change include:
Only keys missing the prefix were updated.
Keys already using a fully-qualified name were left unchanged.
No functional behavior is modified.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14030
How was this patch tested?
All GitHub CI checks passed successfully. No functional behavior is changed.