Gargi-jais11 commented on code in PR #10109:
URL: https://github.com/apache/ozone/pull/10109#discussion_r3146882314
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingTask.java:
##########
@@ -139,24 +140,38 @@ public BackgroundTaskResult call() throws Exception {
private ContainerBackgroundTaskResult handleDeleteTask() throws Exception {
ContainerBackgroundTaskResult crr;
- final Container container = ozoneContainer.getContainerSet()
- .getContainer(containerData.getContainerID());
+ ContainerSet cs = ozoneContainer.getContainerSet();
+ final Container container =
cs.getContainer(containerData.getContainerID());
+ if (container == null) {
+ return new ContainerBackgroundTaskResult();
+ }
container.writeLock();
- File dataDir = new File(containerData.getChunksPath());
- long startTime = Time.monotonicNow();
- // Scan container's db and get list of under deletion blocks
- try (DBHandle meta = BlockUtils.getDB(containerData, conf)) {
- if (containerData.hasSchema(SCHEMA_V1)) {
- crr = deleteViaSchema1(meta, container, dataDir, startTime);
- } else if (containerData.hasSchema(SCHEMA_V2)) {
- crr = deleteViaSchema2(meta, container, dataDir, startTime);
- } else if (containerData.hasSchema(SCHEMA_V3)) {
- crr = deleteViaSchema3(meta, container, dataDir, startTime);
- } else {
- throw new UnsupportedOperationException(
- "Only schema version 1,2,3 are supported.");
+ try {
+ // Re-fetch AFTER lock to verify the container is not having stale
references while waiting for lock.
+ Container<?> current = cs.getContainer(containerData.getContainerID());
+ if (current == null || current.getContainerData() != containerData) {
+ LOG.warn("Container {} reference is stale. Aborting delete. SCM will
retry with" +
+ " the current replica.", containerData.getContainerID());
+ return new ContainerBackgroundTaskResult();
Review Comment:
@sodonnel On second thought, I don't think an explicit retry loop is
necessary here. When this task returns early, `BlockDeletingService` or will
pick up the same container again in its next service interval **(default: 60 s,
configurable via ozone.block.deleting.service.interval)** and construct a fresh
BlockDeletingTask with the current ContainerData from ContainerSet — which at
that point will be the post-move replica. That is effectively the same retry,
without added complexity inside the task itself.
Adding an in-task retry would require re-fetching ContainerData, the pending
block list, schema version, and chunk paths from the new container —
essentially re-creating what the service already does automatically on its next
run. The only difference is a delay of at most one service interval, which is
acceptable for a background block-deletion task.
I will change the `LOG.warn` to `LOG.info` and add a note saying the
container will be picked up in the next service interval, to make the intent
clear.
Please do suggest if it makes sense or am I missing something.
--
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]
