You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/25 22:29:18 UTC

[GitHub] [pulsar] merlimat opened a new pull request #12190: PIP-45: On LockResource.updateValue() revalidate the lock if needed

merlimat opened a new pull request #12190:
URL: https://github.com/apache/pulsar/pull/12190


   ### Motivation
   
   If we're trying to update the value for an existing lock, we should trigger an immediate revalidation of the lock instead of simply failing the update operation. 
   
   The update can then go through if the lock is successfully revalidated.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] eolivelli merged pull request #12190: PIP-45: On LockResource.updateValue() revalidate the lock if needed

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #12190:
URL: https://github.com/apache/pulsar/pull/12190


   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on a change in pull request #12190: PIP-45: On LockResource.updateValue() revalidate the lock if needed

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12190:
URL: https://github.com/apache/pulsar/pull/12190#discussion_r718727890



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/ResourceLockImpl.java
##########
@@ -218,18 +205,22 @@ synchronized void lockWasInvalidated() {
         if (revalidateAfterReconnection) {
             revalidateAfterReconnection = false;
             log.warn("Revalidate lock at {} after reconnection", path);
-            return revalidate();
+            return revalidate(value);
         } else {
             return CompletableFuture.completedFuture(null);
         }
     }
 
-    synchronized CompletableFuture<Void> revalidate() {
+    synchronized CompletableFuture<Void> revalidate(T newValue) {
         return store.get(path)
                 .thenCompose(optGetResult -> {
                     if (!optGetResult.isPresent()) {
                         // The lock just disappeared, try to acquire it again
-                        return acquireWithNoRevalidation()
+                        synchronized (ResourceLockImpl.this) {

Review comment:
       Makes sense. Done 




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on a change in pull request #12190: PIP-45: On LockResource.updateValue() revalidate the lock if needed

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12190:
URL: https://github.com/apache/pulsar/pull/12190#discussion_r718727890



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/ResourceLockImpl.java
##########
@@ -218,18 +205,22 @@ synchronized void lockWasInvalidated() {
         if (revalidateAfterReconnection) {
             revalidateAfterReconnection = false;
             log.warn("Revalidate lock at {} after reconnection", path);
-            return revalidate();
+            return revalidate(value);
         } else {
             return CompletableFuture.completedFuture(null);
         }
     }
 
-    synchronized CompletableFuture<Void> revalidate() {
+    synchronized CompletableFuture<Void> revalidate(T newValue) {
         return store.get(path)
                 .thenCompose(optGetResult -> {
                     if (!optGetResult.isPresent()) {
                         // The lock just disappeared, try to acquire it again
-                        return acquireWithNoRevalidation()
+                        synchronized (ResourceLockImpl.this) {

Review comment:
       Makes sense. Done 




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] eolivelli commented on a change in pull request #12190: PIP-45: On LockResource.updateValue() revalidate the lock if needed

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12190:
URL: https://github.com/apache/pulsar/pull/12190#discussion_r716409181



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/ResourceLockImpl.java
##########
@@ -218,18 +205,22 @@ synchronized void lockWasInvalidated() {
         if (revalidateAfterReconnection) {
             revalidateAfterReconnection = false;
             log.warn("Revalidate lock at {} after reconnection", path);
-            return revalidate();
+            return revalidate(value);
         } else {
             return CompletableFuture.completedFuture(null);
         }
     }
 
-    synchronized CompletableFuture<Void> revalidate() {
+    synchronized CompletableFuture<Void> revalidate(T newValue) {
         return store.get(path)
                 .thenCompose(optGetResult -> {
                     if (!optGetResult.isPresent()) {
                         // The lock just disappeared, try to acquire it again
-                        return acquireWithNoRevalidation()
+                        synchronized (ResourceLockImpl.this) {

Review comment:
       May it be useful to have a synchonized setVersion() method to reduce code duplication and it will make code more readable?




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org