@dormando, The final version I can think of. It has one *fundamental flaw*, 
memcache.add can also be False because server is down. Now I am interfering 
with productivity at the cost of stopping errors. So I need to do all this 
only if memcache client is up and running, else don't worry about locks. 
That code is still not added.

I have loads of time to get a code in, so I wanted to review all the 
possibilities and also learn some stuff while at it.

Can you review it when you get some time? Maybe someone else can help too 
...

[I think the Ghetto Locking is more appropriate if I didn't have the "Cart" 
concept] but since we have it that way ... I feel, I should do it 
different. Also that example doesn't check if file still exists which is 
also important.

def addToCart(filename, username):
    ableToLock = memcache.add(filename, username)
    if ableToLock:
        # ableToLock can happen if the file is still present
        # or if it was already processed.
        if os.file.ispath(filename):
            # I have a lock and file exists. Think of Cart as a
            # JS Object from where you can pick items to "process".
            return "Added To Cart"
        else:
            # I have a lock but looks like file was processed already.
            # So removing the residual "key" created.
            memcache.delete(filename)
            return "Processed by another user." 
    else:
        # "add" can also fail if remote server is down,
        # but now we are not handling that now. It will
        # block the user's ability to process anything.
        user = memcache.get(filename)
        if user and os.file.ispath(filename):
           # I try my best to show the user processing it.
           print "Being processed by %s" % user
        else:
           # But lost the race to find that.
           print "Processed by another user."

def process(source):
    shutil.move(source, destination)
    # filename => source 
    memcache.delete(filename)




On Monday, June 6, 2016 at 11:29:12 PM UTC+5:30, Nishant Varma wrote:
>
> One more enhancement! I am also reviewing it in 
> http://codereview.stackexchange.com/questions/131239/function-to-lock-a-file-using-memcache
>
> def addToCart(filename):
>     ableToLock = memcache.add(filename, username)
>     if ableToLock:
>         if os.file.ispath(filename)
>             # I have a lock and file exists.
>             return "Added To Cart"
>         else:
>             # I have a lock but looks like file was processed already.
>             memcache.delete(filename)
>             return "Processed by another user." 
>     else:
>         user = memcache.get(filename)
>         if user:
>            # I try my best to show the user processing it.
>            print "Bring processed by %s" % user
>         else:
>            # Lost the race to find that.
>            print "Already processed."
>
> def process(source):
>     shutil.move(source, destination)
>     # filename => source 
>     memcache.delete(filename)
>
>
> On Monday, June 6, 2016 at 7:07:21 PM UTC+5:30, Nishant Varma wrote:
>>
>> Enhanced it a bit to to check if the file is really present as well 
>> before I proceed ... Btw this has to be done after obtaining the lock 
>> otherwise again race condition .... i.e not fileExists and ableToLock 
>> ableToLock and fileExists is the order.
>>
>> def addToCart(filename):
>>     ableToLock = memcache.add(filename, username)
>>     fileExists = os.file.ispath(filename)
>>     if ableToLock:
>>         if fileExists:
>>             return "Added To Cart"
>>         else:
>>             memcache.delete(filename)
>>             return "Processed by another user." 
>>     else:
>>         if fileExists:
>>             return "Processed by another user."
>>         else:
>>             user = memcache.get(filename)
>>             return "%s is Processing" % user
>>
>> I guess this should cover our scenario. Do you have any thoughts?
>>
>>
>> On Monday, June 6, 2016 at 5:20:28 PM UTC+5:30, Nishant Varma wrote:
>>>
>>> key = filename 
>>>> item = memcli:get(key) 
>>>> if (! defined item) { 
>>>>   if (memcli:add(key . "_lock", lock_timeout_time, my_admin_username)) 
>>>> { 
>>>>      [etc] 
>>>>   } else { 
>>>>      # lost the race, handle how you want 
>>>>   } 
>>>> } else if (item.value == my_admin_username) { 
>>>>   # good to go for that future request 
>>>> } 
>>>
>>>
>>>
>>> My scenario is files are displayed from a certain folder in an 
>>> application. Each users has a "cart" where they can add files they are 
>>> working upon. They are going to remove it from the cart 
>>> (memcache.set(filename, None) or memcache.delete(filename)) or process it 
>>> from the cart (i.e move it to a DMS and archive it to another folder and 
>>> memcache.delete(filename)).
>>>
>>> In the example pattern you have given,
>>>
>>> 1) The locking and the action are both happening together. However, we 
>>> want lock to happen like picking the item a cart (i.e memcache.add, if it 
>>> fails it doesn't go to his cart) when the user clicks on the file. If items 
>>> are there in his cart, he can do anything he wants. So things like 
>>> "lock_timeout" is not really needed for us I guess because end user decides 
>>> when to remove also. [There is a flaw here what if someone manipulates DOM 
>>> - but we don't worry that scenario, yours actually takes care of that since 
>>> validation and action if combined]
>>>
>>> 2) You are creating a different key called filename + lock which feels a 
>>> bit redundant in my case because I can achieve everything I want by 
>>> entering the key as filename and like you said username as the value.
>>>
>>> 3) You are also having an additional get which is nice especially 
>>> validating same user. It is something I need to think if I should support 
>>> though. Usually the same user, if he logs in from other machine we don't 
>>> need to support it. 
>>>
>>> Overall given my simplest scenario, wont this simple api just work:
>>>
>>> def addToCart(filename):
>>>     if memcache.add(filename, username):
>>>         return True
>>>     else:
>>>        return False
>>>
>>> API is as simple addToCart(filename).
>>>
>>> I felt that this simplest "add" works for me,  .. What are you thoughts?
>>>
>>> Also my get set was as simple as this, the more reason I felt the change 
>>> should simple as well. Usually that happens.
>>>
>>> Thanks,
>>> Nishant
>>>
>>>
>>> On Sunday, June 5, 2016 at 1:30:37 AM UTC+5:30, Dormando wrote:
>>>>
>>>> The pattern is identical between the one in the wiki and yours. Simply 
>>>> move the delete of the key until you're done using the lock, which 
>>>> would 
>>>> be in a separate request. 
>>>>
>>>> In your case, you would probably set the contents of the key to be the 
>>>> name of the user who has it locked. 
>>>>
>>>> In the original pseudocode: 
>>>> key  = "expensive_frontpage_item" 
>>>> item = memcli:get(key) 
>>>> if (! defined item) { 
>>>>     # Oh crap, we have to recache it! 
>>>>     # Give us 60 seconds to recache the item. 
>>>>     if (memcli:add(key . "_lock", 60)) { 
>>>>         item = fetch_expensive_thing_from_database 
>>>>         memcli:add(key, item, 86400) 
>>>>         memcli:delete(key . "_lock") 
>>>>     } else { 
>>>>         # Lost the race. We can do any number of things: 
>>>>         # - short sleep, then re-fetch. 
>>>>         # - try the above a few times, then slow-fetch and return the 
>>>> item 
>>>>         # - show the user a page without this expensive content 
>>>>         # - show some less expensive content 
>>>>         # - throw an error 
>>>>     } 
>>>> } 
>>>> return item 
>>>>
>>>> In yours: 
>>>> key = filename 
>>>> item = memcli:get(key) 
>>>> if (! defined item) { 
>>>>   if (memcli:add(key . "_lock", lock_timeout_time, my_admin_username)) 
>>>> { 
>>>>      [etc] 
>>>>   } else { 
>>>>      # lost the race, handle how you want 
>>>>   } 
>>>> } else if (item.value == my_admin_username) { 
>>>>   # good to go for that future request 
>>>> } 
>>>>
>>>> Then when you're done holding the lock, delete the key. 
>>>>
>>>> On Sat, 4 Jun 2016, Nishant Varma wrote: 
>>>>
>>>> > I am reading 
>>>> https://github.com/memcached/memcached/wiki/ProgrammingTricks#ghetto-central-locking,
>>>>  
>>>> it seems to deal with a slightly different lock scenario of getting some 
>>>> > expensive item from Database to avoid "Stampeding" 
>>>> > In my case its slightly different lock that I need. I show regular 
>>>> files from a folder in a web application to many users. So, to "lock" a 
>>>> file using memcache isn't this 
>>>> > simple API sufficient or I still need that pattern :-)? 
>>>> > def access(filename): 
>>>> >      if memcache.add(filename, timestamp): 
>>>> >         return "Access Granted. Lock Obtained" # Normally this 
>>>> results in checking HTML checkbox against the filename so User can do 
>>>> actions with that/ 
>>>> >      else: 
>>>> >         return "Access Denied" # Normally this leads to an alert 
>>>> saying that someone else is working on this. 
>>>> > 
>>>> > Isn't this simple API using add good enough in my case? I am sorry if 
>>>> I am repeating this, but I could not really relate the "fetching expensive 
>>>> item from Database" to my 
>>>> > scenario which is why I even wrote a simple script to test the 
>>>> validity of the claim etc. 
>>>> > 
>>>> > Can you please let me know? 
>>>> > 
>>>> > 
>>>> > On Saturday, June 4, 2016 at 6:42:35 PM UTC+5:30, Nishant Varma 
>>>> wrote: 
>>>> >       Excellent I rely on you. I guess this is the reason you say I 
>>>> am over-engineering this problem. Makes sense :-) I will again check the 
>>>> link you gave me. I will go 
>>>> >       through the documentation this weekend. 
>>>> > 
>>>> >       On Saturday, June 4, 2016 at 1:33:04 PM UTC+5:30, Dormando 
>>>> wrote: 
>>>> >             Hey, 
>>>> > 
>>>> >             You really don't need to test this: I'm telling you 
>>>> flatly, as an author 
>>>> >             of this software and all of the documentation for it, 
>>>> that you should 
>>>> >             absolutely not rely on that pattern. I'm trying to save 
>>>> you some time. 
>>>> > 
>>>> >             The pattern that is slightly better is written explicitly 
>>>> in pseudocode in 
>>>> >             the link I gave you several times in the issue. Please 
>>>> use it. 
>>>> > 
>>>> >             Thanks, 
>>>> >             -Dormando 
>>>> > 
>>>> >             On Fri, 3 Jun 2016, Nishant Varma wrote: 
>>>> > 
>>>> >             > Can anyone help me peer review this script 
>>>> https://gist.github.com/varmanishant/0129286d41038cc21471652a6460a5ff 
>>>> that demonstrate potential problems 
>>>> >             with get set if it is used 
>>>> >             > to implement distributed locking. I was suggested to 
>>>> modify from get set to add in this thread 
>>>> https://github.com/memcached/memcached/issues/163. 
>>>> >             However I wanted a small 
>>>> >             > simulation to demonstrate this. 
>>>> >             > 
>>>> >             > -- 
>>>> >             > 
>>>> >             > --- 
>>>> >             > You received this message because you are subscribed to 
>>>> the Google Groups "memcached" group. 
>>>> >             > To unsubscribe from this group and stop receiving 
>>>> emails from it, send an email to [email protected]. 
>>>> >             > For more options, visit 
>>>> https://groups.google.com/d/optout. 
>>>> >             > 
>>>> >             > 
>>>> > 
>>>> > 
>>>> > This e-mail message (including any attachments) may contain 
>>>> information that is confidential, protected by the attorney-client or 
>>>> other 
>>>> applicable privileges, or otherwise 
>>>> > comprising non-public information. This message is intended to be 
>>>> conveyed only to the designated recipient(s). If you have any reason to 
>>>> believe you are not an intended 
>>>> > recipient of this message, please notify the sender by replying to 
>>>> this message and then deleting it from your system. Any use, 
>>>> dissemination, 
>>>> distribution, or reproduction of 
>>>> > this message by unintended recipients is not authorized and may be 
>>>> unlawful. 
>>>> > 
>>>> > -- 
>>>> > 
>>>> > --- 
>>>> > You received this message because you are subscribed to the Google 
>>>> Groups "memcached" group. 
>>>> > To unsubscribe from this group and stop receiving emails from it, 
>>>> send an email to [email protected]. 
>>>> > For more options, visit https://groups.google.com/d/optout. 
>>>> > 
>>>> >
>>>
>>>
-- 
This e-mail message (including any attachments) may contain information 
that is confidential, protected by the attorney-client or other applicable 
privileges, or otherwise comprising non-public information. This message is 
intended to be conveyed only to the designated recipient(s). If you have 
any reason to believe you are not an intended recipient of this message, 
please notify the sender by replying to this message and then deleting it 
from your system. Any use, dissemination, distribution, or reproduction of 
this message by unintended recipients is not authorized and may be unlawful.

-- 

--- 
You received this message because you are subscribed to the Google Groups 
"memcached" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to