Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public final class BlockDeletingServiceMetrics {
@Metric(about = "The total number of blocks pending for processing.")
private MutableGaugeLong totalPendingBlockCount;

@Metric(about = "The total number of transactions witch was failed " +
"cause by wait Container lock timeout.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: witch -> which

private MutableGaugeLong totalLockTimeoutTransactionCount;

private BlockDeletingServiceMetrics() {
}

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

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

public long getSuccessCount() {
return successCount.value();
}
Expand All @@ -114,6 +122,10 @@ public long getTotalPendingBlockCount() {
return totalPendingBlockCount.value();
}

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

@Override
public String toString() {
StringBuffer buffer = new StringBuffer();
Expand All @@ -123,7 +135,9 @@ public String toString() {
.append("outOfOrderDeleteBlockTransactionCount = "
+ outOfOrderDeleteBlockTransactionCount.value()).append("\t")
.append("totalPendingBlockCount = "
+ totalPendingBlockCount.value()).append("\t");
+ totalPendingBlockCount.value()).append("\t")
.append("totalLockTimeoutTransactionCount = "
+ totalLockTimeoutTransactionCount.value()).append("\t");
return buffer.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,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
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
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
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
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
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 @@ -559,6 +577,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 @@ -226,7 +226,8 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails,
.addHandler(new CloseContainerCommandHandler())
.addHandler(new DeleteBlocksCommandHandler(getContainer(),
conf, dnConf.getBlockDeleteThreads(),
dnConf.getBlockDeleteQueueLimit()))
dnConf.getBlockDeleteQueueLimit(),
dnConf.getBlockDeleteCommandHandleLockTimeoutMs()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: pass DatanodeConfiguration to DeleteBlocksCommandHandler instead of individual values.

.addHandler(new ReplicateContainerCommandHandler(conf, supervisor,
pullReplicatorWithMetrics, pushReplicatorWithMetrics))
.addHandler(reconstructECContainersCommandHandler)
Expand Down
Loading