Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@
<Class name="org.apache.hadoop.ozone.container.common.statemachine.TestStateContext" />
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
</Match>
<Match>
<Class name="org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestDeleteBlocksCommandHandler" />
<Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
</Match>
<Match>
<Class name="org.apache.hadoop.ozone.container.common.volume.TestVolumeSet" />
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public final class BlockDeletingServiceMetrics {
@Metric(about = "The total number of Container chosen to be deleted.")
private MutableGaugeLong totalContainerChosenCount;

@Metric(about = "The total number of transactions which was failed " +

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.

which was failed cause by wait Container lock timeout -> which failed due to container lock wait timeout

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.

Done.

"cause by wait Container lock timeout.")
private MutableGaugeLong totalLockTimeoutTransactionCount;

private BlockDeletingServiceMetrics() {
}

Expand Down Expand Up @@ -140,6 +144,10 @@ public void setTotalPendingBlockCount(long count) {
this.totalPendingBlockCount.set(count);
}

public void incrTotalLockTimeoutTransactionCount() {
totalLockTimeoutTransactionCount.incr();
}

public long getSuccessCount() {
return successCount.value();
}
Expand Down Expand Up @@ -172,6 +180,10 @@ public long getTotalContainerChosenCount() {
return totalContainerChosenCount.value();
}

public long getTotalLockTimeoutTransactionCount() {
return totalLockTimeoutTransactionCount.value();
}

@Override
public String toString() {
StringBuffer buffer = new StringBuffer();
Expand All @@ -195,7 +207,9 @@ public String toString() {
.append("receivedBlockCount = "
+ receivedBlockCount.value()).append("\t")
.append("markedBlockCount = "
+ markedBlockCount.value()).append("\t");
+ markedBlockCount.value()).append("\t")
.append("totalLockTimeoutTransactionCount = "
+ totalLockTimeoutTransactionCount.value()).append("\t");
return buffer.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,24 @@ public class DatanodeConfiguration {
private long recoveringContainerScrubInterval =
Duration.ofMinutes(10).toMillis();

/**
* Timeout for the thread used to process the delete block command
* to wait for the container lock.
* It takes about 200ms to open a RocksDB with HDD media.
* Set the default value to 100ms, so that after one times retry
* after waiting timeout, the hold time spent waiting for the lock
* is not greater than the time spent operating RocksDB
*/
@Config(key = "block.delete.command.handle.lock.timeout",

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.

Suggest change the property name from block.delete.command.handle.lock.timeout ->
block.delete.max.lock.wait.timeout

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.

@xichen01 , is the slow RocksDB open a major cause behind this patch? I have one question, it takes 200ms to open a rocksdb, you set this timeout to 100ms, and retry once in the block deletion logic, why not set the default value to 200ms, with a simpler handler logic?

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.

Will this property be a reconfigurable property?

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.

This comment may have caused some misunderstanding, I'll modify it.

The retry does not happen immediately, it waits until all DeleteBlockTransactions have been processed, so the retry may happen after a few seconds or tens of seconds.

Slow RocksDB open (Schema V2 only), rebalancer, BlockDeletingService, etc, may cause here cannot get the lock, retry to increase the chance of success.

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.

@xichen01 , thanks for the further info.

defaultValue = "100ms",
type = ConfigType.TIME,
tags = { DATANODE, ConfigTag.DELETION},
description = "Timeout for the thread used to process the delete" +
" block command to wait for the container lock."
)
private long blockDeleteCommandHandleLockTimeoutMs =
Duration.ofMillis(100).toMillis();

public Duration getBlockDeletionInterval() {
return Duration.ofMillis(blockDeletionInterval);
}
Expand Down Expand Up @@ -573,6 +591,10 @@ public int getBlockDeleteQueueLimit() {
return blockDeleteQueueLimit;
}

public long getBlockDeleteCommandHandleLockTimeoutMs() {
return blockDeleteCommandHandleLockTimeoutMs;
}

public void setBlockDeleteQueueLimit(int queueLimit) {
this.blockDeleteQueueLimit = queueLimit;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,7 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails,
commandDispatcher = CommandDispatcher.newBuilder()
.addHandler(new CloseContainerCommandHandler())
.addHandler(new DeleteBlocksCommandHandler(getContainer(),
conf, dnConf.getBlockDeleteThreads(),
dnConf.getBlockDeleteQueueLimit()))
conf, dnConf))
.addHandler(new ReplicateContainerCommandHandler(conf, supervisor,
pullReplicatorWithMetrics, pushReplicatorWithMetrics))
.addHandler(reconstructECContainersCommandHandler)
Expand Down
Loading