[jira] [Commented] (CURATOR-592) Provide a callback that is fired after the InterProcessMutex creates its ephemeral node

["Mohamed Mohsen (Jira)" <ji...@apache.org>]

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

Mohamed Mohsen commented on CURATOR-592:
----------------------------------------

I created [a PR|https://github.com/apache/curator/pull/384] but it has one problem and that is I couldn't easily implement this feature for [InterProcessMultiLock|https://github.com/apache/curator/pull/384/files#diff-da2d90687c51d1253ff275f706919977645c39e7cffcba924d7e45e7fb6bac55] class.

The reason is that the class acquires its underlying locks in a single thread. So the underlying locks are acquired sequentially, and passing the callback to them, to be fired when the underlying lock's ephemeral node is created, is just useless because successive locks are blocked from requesting their locks until previous locks are acquired.

To fire the lock creation callback for all the underlying locks, those locks acquisition will have to be performed in separate threads. But if we do that, we won't be able to release those underlying locks because the threads that acquired those locks are probably dead, and only those threads can release those locks.

In other words, that class will require a significant amount of changes.

Frankly, I don't care about this class now but I can't just ignore it without the community's approval. Or maybe someone else has a better/easier idea about how to handle this.

The useless approach, which is what I did in the linked PR, I simple passed the callback to each underlying lock acquisition method. This means that the callback will be called sequentially for each lock, and after the acquisition of previous locks.

Another approach is to make sure that no one passes in a callback to that class by throwing an exception if an instance is passed in as a call back to the `acquire` method. So developers will have to always pass in a null, or just use the signature that doesn't accept the callback method. I believe that's a very good solution for everyone.

The best approach in my perspective is to revamp that class to acquire and release each underlying lock in a separate thread. But is that currently necessary? I mean, I'm not breaking anything.

> Provide a callback that is fired after the InterProcessMutex creates its ephemeral node
> ---------------------------------------------------------------------------------------
>
>                 Key: CURATOR-592
>                 URL: https://issues.apache.org/jira/browse/CURATOR-592
>             Project: Apache Curator
>          Issue Type: Improvement
>          Components: Recipes
>    Affects Versions: 5.1.0
>            Reporter: Mohamed Mohsen
>            Priority: Major
>              Labels: Concurrency
>             Fix For: TBD
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> {{InterProcessMutex}} performs two significant steps to [acquire|https://curator.apache.org/apidocs/org/apache/curator/framework/recipes/locks/InterProcessMutex.html#acquire()] its lock:
>  # Creates an ephemeral sequential node. This practically means that it took a position in the lock queue (i.e. based on its sequence value).
>  # Blocks until the lock is acquired.
> This improvement is about providing a callback that notifies the caller that the lock order is reserved.
> The use for this, at least for us, is that we needed to differentiate between the two phases for higher concurrency because we can do other actions after knowing that the {{InterProcessMutex}} order is known. Especially since the lock acquisition can take too long, depending on the situation of course.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)