From ed76e0a363091e2232018886c3786dab34d826f5 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 22 May 2024 12:35:24 +0200 Subject: [PATCH 1/4] HDDS-10899. Refactor Lease callbacks --- .../org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java index 3f2d5fbe974..55b74251fb7 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.lease; +import com.google.common.collect.ImmutableList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,7 +46,7 @@ public class LeaseCallbackExecutor implements Runnable { */ public LeaseCallbackExecutor(T resource, List> callbacks) { this.resource = resource; - this.callbacks = callbacks; + this.callbacks = callbacks == null ? ImmutableList.of() : ImmutableList.copyOf(callbacks); } @Override From e25147c80dfb4c4fe740cad3e6f9718d4678cbf9 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Wed, 22 May 2024 12:36:34 +0200 Subject: [PATCH 2/4] change Lease to have at most one callback --- .../org/apache/hadoop/ozone/lease/Lease.java | 26 ++++++++----------- .../ozone/lease/LeaseCallbackExecutor.java | 15 +++++------ .../hadoop/ozone/lease/LeaseManager.java | 3 +-- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/Lease.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/Lease.java index ccf33019aeb..727d7922539 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/Lease.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/Lease.java @@ -19,9 +19,6 @@ import org.apache.hadoop.util.Time; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicLong; @@ -49,9 +46,9 @@ public class Lease { private boolean expired; /** - * Functions to be called in case of timeout. + * Function to be called in case of timeout. */ - private List> callbacks; + private Callable callback; /** @@ -63,11 +60,7 @@ public class Lease { * Lease lifetime in milliseconds */ public Lease(T resource, long timeout) { - this.resource = resource; - this.leaseTimeout = new AtomicLong(timeout); - this.callbacks = Collections.synchronizedList(new ArrayList<>()); - this.creationTime = Time.monotonicNow(); - this.expired = false; + this(resource, timeout, null); } /** @@ -81,8 +74,11 @@ public Lease(T resource, long timeout) { * Callback registered to be triggered when lease expire */ public Lease(T resource, long timeout, Callable callback) { - this(resource, timeout); - callbacks.add(callback); + this.resource = resource; + this.leaseTimeout = new AtomicLong(timeout); + this.callback = callback; + this.creationTime = Time.monotonicNow(); + this.expired = false; } /** @@ -176,15 +172,15 @@ public String toString() { * * @return callbacks to be executed */ - List> getCallbacks() { - return callbacks; + Callable getCallback() { + return callback; } /** * Expires/Invalidates the lease. */ void invalidate() { - callbacks = null; + callback = null; expired = true; } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java index 55b74251fb7..80ca937c006 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseCallbackExecutor.java @@ -17,11 +17,9 @@ package org.apache.hadoop.ozone.lease; -import com.google.common.collect.ImmutableList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.List; import java.util.concurrent.Callable; /** @@ -34,19 +32,19 @@ public class LeaseCallbackExecutor implements Runnable { LoggerFactory.getLogger(LeaseCallbackExecutor.class); private final T resource; - private final List> callbacks; + private final Callable callback; /** * Constructs LeaseCallbackExecutor instance with list of callbacks. * * @param resource * The resource for which the callbacks are executed - * @param callbacks - * Callbacks to be executed by this executor + * @param callback + * Callback to be executed by this executor */ - public LeaseCallbackExecutor(T resource, List> callbacks) { + public LeaseCallbackExecutor(T resource, Callable callback) { this.resource = resource; - this.callbacks = callbacks == null ? ImmutableList.of() : ImmutableList.copyOf(callbacks); + this.callback = callback; } @Override @@ -54,7 +52,7 @@ public void run() { if (LOG.isDebugEnabled()) { LOG.debug("Executing callbacks for lease on {}", resource); } - for (Callable callback : callbacks) { + if (callback != null) { try { callback.call(); } catch (Exception e) { @@ -63,5 +61,4 @@ public void run() { } } } - } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java index 7dfcf3eb8c8..dff6039ae05 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java @@ -268,10 +268,9 @@ public void run() { long remainingTime = lease.getRemainingTime(); if (remainingTime <= 0) { //Lease has timed out - List> leaseCallbacks = lease.getCallbacks(); release(resource); executorService.execute( - new LeaseCallbackExecutor<>(resource, leaseCallbacks)); + new LeaseCallbackExecutor<>(resource, lease.getCallback())); } else { sleepTime = Math.min(remainingTime, sleepTime); } From 78b8635115ab1610e721f2cfcff2f80b4582c64c Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 22 May 2024 12:40:42 +0200 Subject: [PATCH 3/4] remove unused import --- .../main/java/org/apache/hadoop/ozone/lease/LeaseManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java index dff6039ae05..6cb04ea3688 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java @@ -17,7 +17,6 @@ package org.apache.hadoop.ozone.lease; -import java.util.List; import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; From 7bf46a72d2298cd3e2d28fa4efa5c39a0226ddde Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 22 May 2024 13:41:55 +0200 Subject: [PATCH 4/4] get callback before release --- .../main/java/org/apache/hadoop/ozone/lease/LeaseManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java index 6cb04ea3688..bb4ccc1ac1e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lease/LeaseManager.java @@ -267,9 +267,10 @@ public void run() { long remainingTime = lease.getRemainingTime(); if (remainingTime <= 0) { //Lease has timed out + Callable leaseCallback = lease.getCallback(); release(resource); executorService.execute( - new LeaseCallbackExecutor<>(resource, lease.getCallback())); + new LeaseCallbackExecutor<>(resource, leaseCallback)); } else { sleepTime = Math.min(remainingTime, sleepTime); }