sumitagrawl commented on code in PR #7314:
URL: https://github.com/apache/ozone/pull/7314#discussion_r1856139673
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/BlockDeletingService.java:
##########
@@ -149,12 +151,19 @@ public BackgroundTaskQueue getTasks() {
containerBlockInfos = builder.build();
queue.add(containerBlockInfos);
totalBlocks += containerBlockInfo.getNumBlocksToDelete();
+ LOG.info("Queued- Container: {}, deleted blocks: {}",
+ containerBlockInfo.getContainerData().getContainerID(),
containerBlockInfo.getNumBlocksToDelete());
}
metrics.incrTotalBlockChosenCount(totalBlocks);
metrics.incrTotalContainerChosenCount(containers.size());
if (containers.size() > 0) {
- LOG.debug("Queued {} blocks from {} containers for deletion",
- totalBlocks, containers.size());
+ LOG.info("In this iteration, Queued {} blocks from {} containers for
deletion, blocksLimit was {}," +
+ " elapsed time {}ms.", totalBlocks, containers.size(),
blocksLimitPerInterval,
+ Time.monotonicNow() - startTime);
+ if (totalBlocks >= blocksLimitPerInterval) {
+ LOG.warn("Limit for no. of blocks that can be deleted in one
iteration is reached. Current limit: {} = {}",
Review Comment:
this is not a warn scenario, no need have this log.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/BlockDeletingService.java:
##########
@@ -149,12 +151,19 @@ public BackgroundTaskQueue getTasks() {
containerBlockInfos = builder.build();
queue.add(containerBlockInfos);
totalBlocks += containerBlockInfo.getNumBlocksToDelete();
+ LOG.info("Queued- Container: {}, deleted blocks: {}",
+ containerBlockInfo.getContainerData().getContainerID(),
containerBlockInfo.getNumBlocksToDelete());
}
metrics.incrTotalBlockChosenCount(totalBlocks);
metrics.incrTotalContainerChosenCount(containers.size());
if (containers.size() > 0) {
- LOG.debug("Queued {} blocks from {} containers for deletion",
- totalBlocks, containers.size());
+ LOG.info("In this iteration, Queued {} blocks from {} containers for
deletion, blocksLimit was {}," +
Review Comment:
this log is already there as part of
org.apache.hadoop.ozone.container.common.interfaces.ContainerDeletionChoosingPolicyTemplate#chooseContainerForBlockDeletion,
this is redundant log, to be removed.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OpenKeyCleanupService.java:
##########
@@ -224,14 +240,26 @@ public BackgroundTaskResult call() throws Exception {
final OMResponse response = submitRequest(createCommitKeyRequest(b));
if (response != null && response.getSuccess()) {
ozoneManager.getMetrics().incNumOpenKeysHSyncCleaned();
+ if (LOG.isDebugEnabled()) {
+ StringBuilder sb = new StringBuilder();
+ for (CommitKeyRequest.Builder openKey : hsyncKeys) {
+ sb.append(openKey.getKeyArgs().getVolumeName() +
OZONE_URI_DELIMITER +
+ openKey.getKeyArgs().getBucketName() + ": ")
+ .append(openKey.getKeyArgs().getKeyName())
+ .append(", ");
+ }
+ LOG.debug("hsync'ed openKeys committed in current iteration: \n"
+ sb);
+ }
}
});
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Number of expired open keys submitted for deletion: {},"
- + " for commit: {}, elapsed time: {}ms",
- numOpenKeys, numHsyncKeys, Time.monotonicNow() - startTime);
+ LOG.info("Number of expired open keys submitted for deletion: {},"
+ + " for commit: {}, cleanupLimit: {}, elapsed time: {}ms",
+ numOpenKeys, numHsyncKeys, cleanupLimitPerTask, Time.monotonicNow()
- startTime);
+ if ((numOpenKeys + numHsyncKeys) >= cleanupLimitPerTask) {
+ LOG.warn("Limit for no. of openKeys that can be cleaned Up in one
iteration is reached. Current limit: {} = {}",
Review Comment:
This is not a warning, if able to process is good enough.
and This log in redundant, not required, info can be retrieved from
configuration.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/BlockDeletingService.java:
##########
@@ -149,12 +151,19 @@ public BackgroundTaskQueue getTasks() {
containerBlockInfos = builder.build();
queue.add(containerBlockInfos);
totalBlocks += containerBlockInfo.getNumBlocksToDelete();
+ LOG.info("Queued- Container: {}, deleted blocks: {}",
Review Comment:
need avoid be info log, else too much logs. can be debug log.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -222,6 +222,10 @@ public BackgroundTaskResult call() {
getOzoneManager().getKeyManager(),
pendingKeysDeletion.getKeysToModify(), null,
expectedPreviousSnapshotId);
deletedKeyCount.addAndGet(delCount);
+ if (delCount >= getKeyLimitPerTask()) {
+ LOG.warn("Limit for no. of keys that can be deleted in one
iteration is reached. Current limit: {} = {}",
Review Comment:
This is not a warn scenario, can be retrieved from configuration.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java:
##########
@@ -449,13 +453,21 @@ public long optimizeDirDeletesAndSubmitRequest(long
remainNum,
deletedDirsCount.addAndGet(dirNum + subdirDelNum);
movedDirsCount.addAndGet(subDirNum - subdirDelNum);
movedFilesCount.addAndGet(subFileNum);
+ long timeTakenInIteration = Time.monotonicNow() - startTime;
LOG.info("Number of dirs deleted: {}, Number of sub-dir " +
"deleted: {}, Number of sub-files moved:" +
" {} to DeletedTable, Number of sub-dirs moved {} to " +
- "DeletedDirectoryTable, iteration elapsed: {}ms," +
+ "DeletedDirectoryTable, limit per iteration: {}, iteration
elapsed: {}ms, " +
" totalRunCount: {}",
- dirNum, subdirDelNum, subFileNum, (subDirNum - subdirDelNum),
- Time.monotonicNow() - startTime, getRunCount());
+ dirNum, subdirDelNum, subFileNum, (subDirNum - subdirDelNum), limit,
+ timeTakenInIteration, getRunCount());
+ if (remainNum <= 0) {
+ LOG.warn("Limit for no. of directory+files that can be deleted in one
iteration is reached. " +
+ "Current limit: {} = {}",
+ OMConfigKeys.OZONE_PATH_DELETING_LIMIT_PER_TASK,
ozoneManager.getConfiguration()
+ .getInt(OMConfigKeys.OZONE_PATH_DELETING_LIMIT_PER_TASK,
+ OMConfigKeys.OZONE_PATH_DELETING_LIMIT_PER_TASK_DEFAULT));
+ }
Review Comment:
Metrics can do this job, may be this log not required, going with
multi-threaded, it will create more confusion.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java:
##########
@@ -203,11 +204,16 @@ public EmptyTaskResult call() throws Exception {
}
}
LOG.info("Totally added {} blocks to be deleted for"
- + " {} datanodes / {} totalnodes, task elapsed time: {}ms",
+ + " {} datanodes / {} totalnodes, limit per iteration :
{}blocks, task elapsed time: {}ms",
transactions.getBlocksDeleted(),
transactions.getDatanodeTransactionMap().size(),
included.size(),
+ blockDeletionLimit,
Time.monotonicNow() - startTime);
+ if (transactions.getBlocksDeleted() >= blockDeletionLimit) {
+ LOG.warn("Limit for no. of keys that can be deleted in one
iteration is reached. Current limit: {} = {}",
Review Comment:
this is not a warn scenario, no need have this log
--
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]
