[GitHub] [hadoop] ayushtkn commented on a change in pull request #2833: HDFS-15934: Make DirectoryScanner reconcile blocks batch size and int…

2021-05-04 Thread GitBox


ayushtkn commented on a change in pull request #2833:
URL: https://github.com/apache/hadoop/pull/2833#discussion_r625692545



##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
##
@@ -315,6 +316,41 @@ public DirectoryScanner(FsDatasetSpi dataset, 
Configuration conf) {
 
 masterThread =
 new ScheduledThreadPoolExecutor(1, new Daemon.DaemonFactory());
+
+int reconcileBatchSize =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT);
+
+if (reconcileBatchSize < 0) {

Review comment:
   change it to <=0

##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
##
@@ -315,6 +316,41 @@ public DirectoryScanner(FsDatasetSpi dataset, 
Configuration conf) {
 
 masterThread =
 new ScheduledThreadPoolExecutor(1, new Daemon.DaemonFactory());
+
+int reconcileBatchSize =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT);
+
+if (reconcileBatchSize < 0) {
+  LOG.warn("Invalid value configured for " +
+  "dfs.datanode.reconcile.blocks.batch.size, " +
+  "should be greater than 0, Using default.");
+  reconcileBatchSize =
+  DFSConfigKeys.
+  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT;
+}
+
+reconcileBlocksBatchSize = reconcileBatchSize;
+
+long reconcileBatchInterval =
+conf.getTimeDuration(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT,
+TimeUnit.MILLISECONDS);
+
+if (reconcileBatchInterval < 0) {

Review comment:
   change it to <=0




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] ayushtkn commented on a change in pull request #2833: HDFS-15934: Make DirectoryScanner reconcile blocks batch size and int…

2021-04-15 Thread GitBox


ayushtkn commented on a change in pull request #2833:
URL: https://github.com/apache/hadoop/pull/2833#discussion_r614224919



##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
##
@@ -315,6 +316,40 @@ public DirectoryScanner(FsDatasetSpi dataset, 
Configuration conf) {
 
 masterThread =
 new ScheduledThreadPoolExecutor(1, new Daemon.DaemonFactory());
+
+int reconcileBatchSize =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT);
+
+if (reconcileBatchSize < 0) {
+  LOG.warn(
+  "Invalid value configured for 
dfs.datanode.reconcile.blocks.batch.size, " +
+  "should be greater than 0, Using default.");
+  reconcileBatchSize =
+  DFSConfigKeys.
+  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT;
+}
+
+reconcileBlocksBatchSize = reconcileBatchSize;
+
+long reconcileBatchInterval =
+conf.getLong(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT);

Review comment:
   This should use ``conf.getTimeDuration``, If you need some reference can 
check HDFS-15107, so as how to add support for time units.

##
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
##
@@ -873,6 +873,22 @@
   
 
 
+
+  dfs.datanode.reconcile.blocks.batch.size
+  1000
+  HDFS-14476 to enable running reconcile to checkAndUpdate with 
batch,

Review comment:
   No need for jira id in the description, and this doesn't enable/disable, 
it is just to specify the batch size.

##
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
##
@@ -873,6 +873,22 @@
   
 
 
+
+  dfs.datanode.reconcile.blocks.batch.size
+  1000
+  HDFS-14476 to enable running reconcile to checkAndUpdate with 
batch,
+setting this to define batch size.
+  
+
+
+
+  dfs.datanode.reconcile.blocks.batch.interval
+  2000
+  Setting this to define interval between
+batches defined in dfs.datanode.reconcile.blocks.batch.size.
+  

Review comment:
   Some grammatical error, Please rectify 

##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
##
@@ -315,6 +316,40 @@ public DirectoryScanner(FsDatasetSpi dataset, 
Configuration conf) {
 
 masterThread =
 new ScheduledThreadPoolExecutor(1, new Daemon.DaemonFactory());
+
+int reconcileBatchSize =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT);
+
+if (reconcileBatchSize < 0) {
+  LOG.warn(
+  "Invalid value configured for 
dfs.datanode.reconcile.blocks.batch.size, " +
+  "should be greater than 0, Using default.");
+  reconcileBatchSize =
+  DFSConfigKeys.
+  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT;
+}
+
+reconcileBlocksBatchSize = reconcileBatchSize;
+
+long reconcileBatchInterval =
+conf.getLong(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT);
+
+if (reconcileBatchInterval < 0) {
+  LOG.warn(
+  "Invalid value configured for 
dfs.datanode.reconcile.blocks.batch.interval, " +
+  "should be greater than 0, Using default.");
+  reconcileBatchInterval =
+  DFSConfigKeys.
+  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT;
+}
+
+reconcileBlocksBatchInterval = reconcileBatchInterval;
   }

Review comment:
   You need to format this code properly




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] ayushtkn commented on a change in pull request #2833: HDFS-15934: Make DirectoryScanner reconcile blocks batch size and int…

2021-04-03 Thread GitBox


ayushtkn commented on a change in pull request #2833:
URL: https://github.com/apache/hadoop/pull/2833#discussion_r606714508



##
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
##
@@ -873,6 +873,22 @@
   
 
 
+
+  dfs.datanode.reconcile.blocks.batch.size
+  1000
+  Run reconcile to checkAndUpdate with batch,

Review comment:
   can you add some recheck the descriptions for both the configs, the 
first line in. both is same, and doesn't make sense to me, recheck once.

##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
##
@@ -316,6 +317,18 @@ public DirectoryScanner(FsDatasetSpi dataset, 
Configuration conf) {
 
 masterThread =
 new ScheduledThreadPoolExecutor(1, new Daemon.DaemonFactory());
+
+reconcileBlocksBatchSize =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT);
+
+reconcileBlocksBatchInterval =
+conf.getInt(DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL,
+DFSConfigKeys.
+DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT);

Review comment:
   Add a validation for these configs, if ``reconcileBlocksBatchSize`` and 
``reconcileBlocksBatchInterval`` is less than one use default. and add a warn 
log message if these values are incorrect something like:
   Invalid value configured for < config name>, should be greater than 0, Using 
default.
   
   In the end add an Info log for the values being used.
   

##
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
##
@@ -836,6 +836,10 @@
   public static final int DFS_DATANODE_DIRECTORYSCAN_INTERVAL_DEFAULT = 
21600;
   public static final String  DFS_DATANODE_DIRECTORYSCAN_THREADS_KEY = 
"dfs.datanode.directoryscan.threads";
   public static final int DFS_DATANODE_DIRECTORYSCAN_THREADS_DEFAULT = 1;
+  public static final String  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE = 
"dfs.datanode.reconcile.blocks.batch.size";
+  public static final int DFS_DATANODE_RECONCILE_BLOCKS_BATCH_SIZE_DEFAULT 
= 1000;
+  public static final String  DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL = 
"dfs.datanode.reconcile.blocks.batch.interval";
+  public static final int 
DFS_DATANODE_RECONCILE_BLOCKS_BATCH_INTERVAL_DEFAULT = 2000;

Review comment:
   Can we add support for time units?




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org