ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dmitriy Setrakyan (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (IGNITE-59) Support lock, lockAll
Date Thu, 22 Jan 2015 00:24:35 GMT

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

Dmitriy Setrakyan edited comment on IGNITE-59 at 1/22/15 12:23 AM:
-------------------------------------------------------------------

Sergey,

Now, after reviewing the API, I still don't think it is correct. Specifically, I am talking
about {{CacheLock.isLocked()}} and {{CacheLock.isLockedByThread()}} methods. If you look at
JDK ReentrantLock for example, then such methods check the instance of the lock on which they
are invoked on. However, in our case, we check whether the lock was acquired by any thread
and by any Lock instance. I think current Ignite behavior is confusing.

I would like to propose the following:
# Move {{CacheLock.isLocked()}} and {{CacheLock.isLockedByThread()}} to {{IgniteCache}} and
merge them into one method: {{IgniteCache.isLocked(boolean byCurrentThread)}}
# Get rid of {{CacheLock}} interface altogether, since it is now identical to standard {{Lock}}
interface from JDK. We should indicate in the javadoc of {{IgniteCache.lock(...)}} methods
that {{Lock.newCondition()}} is not supported.
# Also, I would like to make sure that if some instance of lock tries to unlock without actually
having acquired the lock in the same thread, it would result in {{IllegateStateException}}.
This can be achieved by maintaining a counter for lock() and a counter for unlock() invocations
and validating these counters for every operation.

Please work on this in a new branch and ask Yakov to review it before merge.


was (Author: dsetrakyan):
Sergey,

Now, after reviewing the API, I still don't think it is correct. Specifically, I am talking
about {{CacheLock.isLocked()}} and {{CacheLock.isLockedByThread()}} methods. If you look at
JDK ReentrantLock for example, then such methods check the instance of the lock on which they
are invoked on. However, in our case, we check whether the lock was acquired by any thread
and by any Lock instance. I think current Ignite behavior is confusing.

I would like to propose the following:
# Move {{CacheLock.isLocked()}} and {{CacheLock.isLockedByThread()}} to {{IgniteCache}} and
merge them into one method: {{IgniteCache.isLocked(boolean byCurrentThread)}}
# Get rid of {{CacheLock}} interface altogether, since it is now identical to standard {{Lock}}
interface from JDK. We should indicate in the javadoc of {{IgniteCache.lock(...)}} methods
that {{Lock.newCondition()}} is not supported.
# Also, I would like to make sure that if some instance of lock tries to unlock without actually
having acquired the lock in the same thread, it would result in {{IllegateStateException}}.
This can be achieved by maintaining a counter for lock() and a counter for unlock() invocations
and validating these counters for every operation.

> Support lock, lockAll
> ---------------------
>
>                 Key: IGNITE-59
>                 URL: https://issues.apache.org/jira/browse/IGNITE-59
>             Project: Ignite
>          Issue Type: Sub-task
>          Components: cache
>            Reporter: Semen Boikov
>            Assignee: Sergey Evdokimov
>             Fix For: sprint-1
>
>
> Implement methods on IgniteCache: 'Lock lock(K key)', 'Lock lockAll(Set<? extends
K> keys)', note: in new design method return Lock instance.



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

Mime
View raw message