[
https://issues.apache.org/jira/browse/CURATOR-592?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17331312#comment-17331312
]
Mohamed Mohsen edited comment on CURATOR-592 at 4/24/21, 8:26 PM:
------------------------------------------------------------------
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.
[~cammckenzie], any suggestions please?
was (Author: mgelbana):
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.
[~cammckenzie], any suggestions?
> 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)
