adoroszlai commented on a change in pull request #142: HDDS-2448 Delete
container command should used a thread pool
URL: https://github.com/apache/hadoop-ozone/pull/142#discussion_r347914057
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -58,4 +63,25 @@ public int getReplicationMaxStreams() {
return replicationMaxStreams;
}
+ @Config(key = "delete.container.threads",
+ type = ConfigType.INT,
+ defaultValue = "2",
+ tags = {DATANODE},
+ description = "The maximum number of threads used to delete containers "
+
+ "on a datanode"
+ )
+ public void setDeleteContainerThreads(int val) {
+ if (val < 1) {
+ LOG.warn("hdds.datanode.delete.container.threads must be greater than" +
+ "zero and was set to {}. Defaulting to {}",
+ val, deleteContainerThreads);
Review comment:
This is great only for the case when config is set during load from XML.
Later, if the config is being set programmatically, we have two (minor) issues:
1. the default value is not the real default, rather the previously set
valid config
2. warning is human-friendly, but irrelevant to programmatic callers
Eg.
```
setDeleteContainerThreads(10);
setDeleteContainerThreads(-1);
```
would log `... Defaulting to 10`.
So I think this could be improved by:
1. creating a constant for the default value
2. using an `Integer` to distinguish between unset and set states
3. logging the warning only if previously unset
4. _consider_ throwing an exception (or silently ignoring invalid values) if
previously already set
(I wanted to mention this for the previous, replication-related PR, but was
late to the party. ;) )
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
