cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paulo Motta (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-11258) Repair scheduling - Resource locking API
Date Thu, 26 May 2016 23:02:12 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-11258?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15303125#comment-15303125
] 

Paulo Motta commented on CASSANDRA-11258:
-----------------------------------------

bq. Sorry for the delay. I've added the check for the existing lease in case we get a WTE
here.

+1 this case seems to be handled correctly now. The rest of the patch looks mostly good, apart
from these minor nits:

* We can probably make the validity check a bit more robust by failing-fast if there is a
different persisted {{expirationTime}} value:
{noformat}
diff --git a/src/java/org/apache/cassandra/scheduling/CasLeaseFactory.java b/src/java/org/apache/cassandra/scheduling/CasLeaseFactory.java
index 5629613..2c34238 100644
--- a/src/java/org/apache/cassandra/scheduling/CasLeaseFactory.java
+++ b/src/java/org/apache/cassandra/scheduling/CasLeaseFactory.java
@@ -410,7 +410,12 @@ public class CasLeaseFactory implements LeaseFactory
         @Override
         public boolean isValid()
         {
-            return !hasExpired() && holdsLeaseUntil(resource).isPresent();
+            if (hasExpired())
+                return false;
+
+            Optional<Long> holdsLeasseUntil = holdsLeaseUntil(resource);
+            return holdsLeasseUntil.isPresent() && holdsLeasseUntil.get().equals(expirationTime);
         }
 
         private boolean hasExpired()
{noformat}
* Replace {{logger.error}} with {{logger.warn}} since all of these errors are recoverable
so the user doesn't get scared by it.
* Add a test scenario for the case where the lease is acquirde, renewed with a shorter TTL,
cancelled and re-acquired shortly after to see if the case we discussed earlier is being correctly
addressed.

bq. I've added an example in the javadoc, hopefully that will make it clearer. I'm a bit unsure
about the "This duration is not added to the current duration." part of the javadoc that I
added, it might confuse more than it contributes, WDYT?

How about renaming {{renew(int duration)}} to {{renew(int newDuration)}} to make this more
explicit? I'm attaching a patch with suggested changes (feel free to modify or use it partially).

> Repair scheduling - Resource locking API
> ----------------------------------------
>
>                 Key: CASSANDRA-11258
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11258
>             Project: Cassandra
>          Issue Type: Sub-task
>            Reporter: Marcus Olsson
>            Assignee: Marcus Olsson
>            Priority: Minor
>         Attachments: newDuration.patch
>
>
> Create a resource locking API & implementation that is able to lock a resource in
a specified data center. It should handle priorities to avoid node starvation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message