adoroszlai commented on code in PR #5611:
URL: https://github.com/apache/ozone/pull/5611#discussion_r1396007459
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java:
##########
@@ -121,7 +123,9 @@ public DeleteBlocksCommandHandler(OzoneContainer container,
dnConf.getBlockDeleteThreads(), threadFactory);
this.deleteCommandQueues =
new LinkedBlockingQueue<>(dnConf.getBlockDeleteQueueLimit());
- handlerThread = new Daemon(new DeleteCmdWorker());
+ long interval = this.conf.getLong(BLOCK_DELETE_COMMAND_WORKER_INTERVAL,
+ BLOCK_DELETE_COMMAND_WORKER_INTERVAL_DEFAULT);
Review Comment:
Get value from `dnConf`:
```suggestion
long interval = dnConf.getBlockDeleteCommandWorkerInterval();
```
(Don't forget to remove unused imports.)
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestDeleteBlocksCommandHandler.java:
##########
@@ -262,6 +263,23 @@ public List<Future<DeleteBlockTransactionExecutionResult>>
answer(
blockDeleteMetrics.getTotalLockTimeoutTransactionCount());
}
+ @Test
+ public void testDeleteCmdWorkerInterval() {
+ OzoneConfiguration tmpConf = new OzoneConfiguration();
+ tmpConf.setLong(BLOCK_DELETE_COMMAND_WORKER_INTERVAL, 3000);
+ OzoneContainer container = Mockito.mock(OzoneContainer.class);
+ DatanodeConfiguration dnConf =
+ tmpConf.getObject(DatanodeConfiguration.class);
+ DeleteBlocksCommandHandler commandHandler =
+ spy(new DeleteBlocksCommandHandler(
+ container, tmpConf, dnConf, "test"));
+
+ Assert.assertEquals(Long.parseLong(tmpConf.get(
+ BLOCK_DELETE_COMMAND_WORKER_INTERVAL)), 3000);
Review Comment:
Since the test has set this value earlier, this assertion does not exercise
the `DeleteCmdWorker` or any other production code.
```suggestion
```
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java:
##########
@@ -184,6 +189,16 @@ public class DatanodeConfiguration extends
ReconfigurableConfig {
)
private int blockDeleteQueueLimit = 5;
+ @Config(key = "block.delete.command.worker.interval",
+ type = ConfigType.LONG,
+ defaultValue = "2000",
+ tags = {DATANODE},
+ description = "The interval between DeleteCmdWorker execution of " +
+ "delete commands."
+ )
+ private long blockDeleteCommandWorkerInterval =
Review Comment:
We can make it more user-friendly and improve type safety by changing this
to `ConfigType.TIME`, which accepts values with unit. With that, default value
can be changed to `2s` and variable type to `Duration`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
