[ 
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)

Reply via email to