jelmini commented on code in PR #760:
URL: https://github.com/apache/jackrabbit-oak/pull/760#discussion_r1049715995


##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -121,6 +132,21 @@ private void refreshLease() {
         }
     }
 
+    private void notifyLockStatusChange(LockStatus renewal) {
+        try {
+            lockStatusChangedCallback.accept(renewal);
+        } catch (RuntimeException e) {
+            log.warn("Exception in lock status change callback", e);
+        }
+    }
+
+    void doRenewLease() throws StorageException {
+        BlobRequestOptions blobRequestOptions = new BlobRequestOptions();
+        blobRequestOptions.setMaximumExecutionTimeInMs(Math.max(INTERVAL - 
renewalInterval - 1, INTERVAL / 2 - 1));

Review Comment:
   Ok, I will add a comment.
   In summary, previously `renewLease()` was called with the default 
`BlobRequestOptions`. However, those defaults  can be set globally for the 
client instance and may be not a good fit for calling `renewLease()` as it 
could block for too long. As the lease is acquired for 60 seconds and 
`renewLease()` is called by default every 30 seconds, I think it should timeout 
after at most 29 seconds (giving 1 sec margin), so that we can call 
`notifyLockStatusChange(LockStatus.RENEWAL_FAILED)` and suspend writes before 
the lease becomes available to be acquired by another instance. 
   BTW, I have just noticed that values should be in millis and thus multiplied 
by 1000...



-- 
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: dev-unsubscr...@jackrabbit.apache.org

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

Reply via email to