[ https://issues.apache.org/jira/browse/IGNITE-638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14951199#comment-14951199 ]
Valentin Kulichenko commented on IGNITE-638: -------------------------------------------- Hi Vladislav, I looked through the code and here are my comments: # I don't see a single test for the new functionality. Please provide good coverage including multi-node tests with restarts. # Example doesn't compile on line 56. Also I think it's a bit too complicated. It's main purpose is to show how to use the API, so it needs to be as simple as possible. # {{GridCacheSemaphoreState.isFair()}} will always return {{false}}, because the constructor that sets it is never used. I believe it's a bug. # You throw an exception if initial number of permits is negative. Java semaphore allows it and I think we should be consistent. # Method names should not be abbreviated. E.g., {{GridCacheSemaphoreState.getCnt()}} should be {{GridCacheSemaphoreState.getCount()}}. # Braces are not always used according to guidelines [1]. For example, see {{DataStructuresProcessor}}, lines 1319-1325. [1] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-BracesandIdentation > Implement IgniteSemaphore data structure > ---------------------------------------- > > Key: IGNITE-638 > URL: https://issues.apache.org/jira/browse/IGNITE-638 > Project: Ignite > Issue Type: Sub-task > Components: data structures > Affects Versions: sprint-9 > Reporter: Dmitriy Setrakyan > Assignee: Vladisav Jelisavcic > Labels: features, newbie > Fix For: 1.5 > > > We need to add {{IgniteSemaphore}} data structure in addition to other data > structures provided by Ignite. {{IgniteSemaphore}} should have similar API to > {{java.util.concurrent.IgniteSemaphore}} class in JDK. > As an example, you can see how > [IgniteCountDownLatch|https://github.com/apache/incubator-ignite/blob/master/modules/core/src/main/java/org/apache/ignite/IgniteCountDownLatch.java] > is implemented in > [GridCacheCountDownLatchImpl|https://github.com/apache/incubator-ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheCountDownLatchImpl.java] > class. > In general we need to have an entity in ATOMIC cache storing number of > permits and allow user threads to block whenever no more permits are > available. -- This message was sent by Atlassian JIRA (v6.3.4#6332)