zookeeper-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Flavio Junqueira <...@apache.org>
Subject Re: Bug in WriteLock recipe
Date Tue, 13 Feb 2018 22:05:21 GMT
Yeah, this problem is also mentioned in that issue 645.

-Flavio

> On 13 Feb 2018, at 23:03, Kathryn Hogg <Kathryn.Hogg@oati.net> wrote:
> 
> Thanks Flavio,
> 
> I'm debugging this and found another issue in the code in findPrefixInChildren()
> 
>        private void findPrefixInChildren(String prefix, ZooKeeper zookeeper, String dir)

>            throws KeeperException, InterruptedException {
>            List<String> names = zookeeper.getChildren(dir, false);
>            for (String name : names) {
>                if (name.startsWith(prefix)) {
>                    id = name;  /*   THIS DOES NOT HAVE THE FULL PATH */
>                    if (LOG.isDebugEnabled()) {
>                        LOG.debug("Found id created last time: " + id);
>                    }
>                    break;
>                }
>            }
>            if (id == null) {
>                id = zookeeper.create(dir + "/" + prefix, data, 
>                        getAcl(), EPHEMERAL_SEQUENTIAL);   /* THIS HAS THE FULL PATH */
> 
>                if (LOG.isDebugEnabled()) {
>                    LOG.debug("Created id: " + id);
>                }
>            }
> 
>        }
> 
> If we find the node in the children, we set id to x-$session-$sequence.  If we create
the znode, id is set to $dir/x-$session-$sequence
> 
> I believe the first case should be 
>  Id = dir + "/" + name;
> 
> -----Original Message-----
> From: Flavio Junqueira [mailto:fpj@apache.org] 
> Sent: Tuesday, February 13, 2018 3:58 PM
> To: user@zookeeper.apache.org
> Subject: Re: Bug in WriteLock recipe
> 
> {External email message: This email is from an external source. Please exercise caution
prior to opening attachments, clicking on links, or providing any sensitive information.}
> 
> You are right that there is a race between getting the children and checking whether
the predecessor is there. If we fail to set a watcher, then we won't try locking again. I
think setting id to null will work because it forces another iteration of the do/while loop,
which checks whether there is still some predecessor, setting a watcher accordingly. The number
of iterations should be finite because we eventually hit the owned znode.
> 
> It might better to add a different condition for clarity, like `while (Id == null ||
watcherNotSet)`. In any case, I'd appreciate if you could chime in and contribute your changes
to https://issues.apache.org/jira/browse/ZOOKEEPER-645 <https://issues.apache.org/jira/browse/ZOOKEEPER-645>.
> 
> -Flavio
> 
>> On 13 Feb 2018, at 22:01, Kathryn Hogg <Kathryn.Hogg@oati.net> wrote:
>> 
>> Hey Flavio,
>> 
>> FYI, I'm on 3.4.11 if that makes a difference.
>> 
>> The problem is
>>     1. In order to get to the exists() call, we've already determined that id is
not null.
>>     2. After stat returns null, we log a message but do not reset stat to null.
>>     3.  The while loop will terminate because id is not null
>> 
>> I think this can fixed by setting id to null if stat returns null. This is similar
to what is done when lessThanMe is not Empty.
>> 
>> Here's an outline of the code I'm talking about (my suggested change is after the
LOG.warn()):
>> 
>> do {
>>  if (id == null) {
>>      ....
>> }
>> If (id != null) {
>>    if (names.IsEmpty()) {
>>        ...
>>        Id = null;
>>    } else {
>>        ....
>>       If (!lessThanMe.isEmpty())
>>                           Stat stat = zookeeper.exists(lastChildId, new LockWatcher());
>>                           if (stat != null) {
>>                               return Boolean.FALSE;
>>                           } else {
>>                               LOG.warn("Could not find the" +
>>                               		" stats for less than me: " + lastChildName.getName());
>>                                /* id = null; */  // id is not null here so the while
loop will terminate 
>>                           }
>>      } else {
>>            if (isOwner()) {
>>                     if (callback != null) {
>>                          callback.lockAcquired();
>>                    }
>>                     return Boolean.TRUE;
>>           }
>>       }
>>   }
>> }
>> }
>> while (id == null);
>> 
>> 
>> -----Original Message-----
>> From: Flavio Junqueira [mailto:fpj@apache.org] 
>> Sent: Tuesday, February 13, 2018 2:45 PM
>> To: user@zookeeper.apache.org
>> Subject: Re: Bug in WriteLock recipe
>> 
>> {External email message: This email is from an external source. Please exercise caution
prior to opening attachments, clicking on links, or providing any sensitive information.}
>> 
>> Hi Kathryn,
>> 
>> Every time that execute method is invoked, it will get children. From your description,
in the case the predecessor node is deleted and stat is null, the next call will not contain
that predecessor znode. Consequently, it won't happen indefinitely. Makes sense?
>> 
>> There is actually an old issue about WriteLock that you may want to have a look:
>> 
>> https://issues.apache.org/jira/browse/ZOOKEEPER-645 <https://issues.apache.org/jira/browse/ZOOKEEPER-645>
>> 
>> Thanks,
>> -Flavio
>> 
>> 
>>> On 13 Feb 2018, at 18:49, Kathryn Hogg <Kathryn.Hogg@oati.net> wrote:
>>> 
>>> I'm actually using the WriteLock from the ZookeeperNetEx C# code but I've verified
that the same issue exists in the Java recipe.  On a busy system, I'm fairly frequently seeing
WriteLock that is never granted to client and gets stuck.
>>> 
>>> What I believe is happening is the lock sets a watch on the request before him
via this code:
>>> 
>>>                          Stat stat = zookeeper.exists(lastChildId, new LockWatcher());
>>>                          if (stat != null) {
>>>                              return Boolean.FALSE;
>>>                          } else {
>>>                              LOG.warn("Could not find the" +
>>>                                                              " stats for less
than me: " + lastChildName.getName());
>>>                          }
>>> 
>>> The problem (as I see it and I'm still fairly new to Zookeeper) is that if the
node represented by lastChildId has been deleted before the call to exists is made, stat will
return null and the watch will only ever be invoked when the znode is created.  And of course
that will never happen.
>>> 
>>> The message is appearing in my log and my watcher for the lock is never invoked.
>>> 
>>> [2018-02-13 16:49:17.905 GMT    WARNING         WriteLock       Could not find
the stats for less than me: /token/SegmentProfileQueueToken/x-72057953399865370-0000000724]
>>> 
>>> I'm not entirely sure of the proper way of fixing this but I think setting
>>>  Id = null;
>>> When stat is null should work.
>>> 
>>> Can someone verify if my analysis is correct?
>>> 
>>> --
>>> Kathryn Hogg
>>> Senior Manager Product Development
>>> Phone: 763.201.2000
>>> Fax: 763.201.5333
>>> Open Access Technology International, Inc.
>>> 3660 Technology Drive NE, Minneapolis, MN 55418
>>> 
>>> CONFIDENTIAL INFORMATION: This email and any attachment(s) contain confidential
and/or proprietary information of Open Access Technology International, Inc.  Do not copy
or distribute without the prior written consent of OATI.  If you are not named a recipient
to the message, please notify the sender immediately and do not retain the message in any
form, printed or electronic.
>>> 
>> 
> 


Mime
View raw message