Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.hadoop.ozone.container.common.transport.server.ratis.XceiverServerRatis;
import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingTask;
import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -133,12 +134,13 @@ public BackgroundTaskQueue getTasks() {
// We must ensure there is no empty container in this result.
// The chosen result depends on what container deletion policy is
// configured.
long startTime = Time.monotonicNow();
int blocksLimitPerInterval = getBlockLimitPerInterval();
List<ContainerBlockInfo> containers =
chooseContainerForBlockDeletion(getBlockLimitPerInterval(),
chooseContainerForBlockDeletion(blocksLimitPerInterval,
containerDeletionPolicy);

BackgroundTask
containerBlockInfos = null;
BackgroundTask containerBlockInfos = null;
long totalBlocks = 0;
for (ContainerBlockInfo containerBlockInfo : containers) {
BlockDeletingTaskBuilder builder =
Expand All @@ -153,8 +155,8 @@ public BackgroundTaskQueue getTasks() {
metrics.incrTotalBlockChosenCount(totalBlocks);
metrics.incrTotalContainerChosenCount(containers.size());
if (containers.size() > 0) {
LOG.debug("Queued {} blocks from {} containers for deletion",
totalBlocks, containers.size());
LOG.info("Queued {} blocks from {} containers for deletion, blocksLimit was {}, elapsed time {}ms.",
totalBlocks, containers.size(), blocksLimitPerInterval, Time.monotonicNow() - startTime);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logs for the total queued is good, but we should also have a similar log for each task per container queued here when it executes. Currently this is just this debug log.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we simply convert the debug log you have pointed out to an info log, or do we want to add a info log in BlockDeletingService pointing out each container + number of blocks deleted for that container when it is added the the queue?

}
} catch (StorageContainerException e) {
LOG.warn("Failed to initiate block deleting tasks, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ public long optimizeDirDeletesAndSubmitRequest(long remainNum,
int remainingBufLimit, KeyManager keyManager,
UUID expectedPreviousSnapshotId) {

long limit = remainNum;
// Optimization to handle delete sub-dir and keys to remove quickly
// This case will be useful to handle when depth of directory is high
int subdirDelNum = 0;
Expand Down Expand Up @@ -452,9 +453,9 @@ public long optimizeDirDeletesAndSubmitRequest(long remainNum,
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, current iteration limit: {}, iteration elapsed: {}ms," +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"DeletedDirectoryTable, current iteration limit: {}, iteration elapsed: {}ms," +
"DeletedDirectoryTable, limit per iteration: {}, iteration elapsed: {}ms," +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also from the Jira description, we should add a WARN log when the service hits its limits, for example:

  • Run time exceeded the interval time
  • Limit per iteration was hit
  • Log the config keys for limit and interval as hints for tuning.

I think this needs to be added to all the deletion services.

" totalRunCount: {}",
dirNum, subdirDelNum, subFileNum, (subDirNum - subdirDelNum),
dirNum, subdirDelNum, subFileNum, (subDirNum - subdirDelNum), limit,
Time.monotonicNow() - startTime, getRunCount());
}
return remainNum;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ public int getPriority() {
@Override
public BackgroundTaskResult call() {
if (shouldRun()) {
isRunningOnAOS.set(true);
final long run = getRunCount().incrementAndGet();
if (LOG.isDebugEnabled()) {
LOG.debug("Running DirectoryDeletingService");
LOG.debug("Running DirectoryDeletingService {}", run);
Comment thread
aryangupta1998 marked this conversation as resolved.
Outdated
}
isRunningOnAOS.set(true);
getRunCount().incrementAndGet();
long dirNum = 0L;
long subDirNum = 0L;
long subFileNum = 0L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CommitKeyRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteOpenKeysRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
Expand All @@ -50,6 +51,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
Expand Down Expand Up @@ -189,7 +191,9 @@ public BackgroundTaskResult call() throws Exception {
if (!shouldRun()) {
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
runCount.incrementAndGet();
final long run = runCount.incrementAndGet();
LOG.debug("Running OpenKeyCleanupService {}", run);

long startTime = Time.monotonicNow();
final ExpiredOpenKeys expiredOpenKeys;
try {
Expand All @@ -212,6 +216,16 @@ public BackgroundTaskResult call() throws Exception {
final OMResponse response = submitRequest(omRequest);
if (response != null && response.getSuccess()) {
ozoneManager.getMetrics().incNumOpenKeysCleaned(numOpenKeys);
if (LOG.isDebugEnabled()) {
StringBuilder sb = new StringBuilder();
for (OpenKeyBucket.Builder openKey : openKeyBuckets) {
sb.append(openKey.getVolumeName() + "." + openKey.getBucketName() + ": ")
.append(openKey.getKeysList().stream().map(OzoneManagerProtocolProtos.OpenKey::getName)
.collect(Collectors.toList()))
.append("\n");
}
LOG.debug("Non-hsync'ed openKeys being deleted in current iteration: \n" + sb);
}
}
}

Expand All @@ -224,15 +238,22 @@ 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() + "." + openKey.getKeyArgs().getBucketName() + ": ")
Comment thread
Tejaskriya marked this conversation as resolved.
Outdated
.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);
final int numKeys = numOpenKeys + numHsyncKeys;
submittedOpenKeyCount.addAndGet(numKeys);
return () -> numKeys;
Expand Down