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

Robert Joseph Evans commented on HADOOP-9046:
---------------------------------------------

It is looking good, and I like how much faster the tests run.

I have a few more comments.

# {code}TODO: should we reject the addition if an action for this 'fs' is 
already present in the queue?{code} If this is an issue then we should file a 
JIRA to fix it, otherwise please remove the TODO. I am leaning more towards 
filing a separate JIRA.
# I don't really like the name lock0 and available0.  I would rather have them 
be named something more descriptive.  This is very minor
# in removeRenewAction we are no longer canceling the token when it is removed. 
I am not sure if that is intentional or not, but it definitely changes the 
functionality of the method. 
# I am not sure that renewerThreadStarted.compairAndSet is that much better 
then isAlive().  isAlive has the problem that if the thread stopped, calling 
add again would result in an IllegalThreadStateException, but compairAndSet has 
the problem that if a new thread cannot be created (Which I have seen happen) 
then only the first call to add will get an exception all other calls to add 
will look like they succeeded but no renewal will ever happen. I would rather 
see errors happen on other calls to add then to have them silently fail.

                
> provide unit-test coverage of class 
> org.apache.hadoop.fs.DelegationTokenRenewer.RenewAction<T>
> ----------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-9046
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9046
>             Project: Hadoop Common
>          Issue Type: Test
>    Affects Versions: 3.0.0, 2.0.3-alpha, 0.23.6
>            Reporter: Ivan A. Veselovsky
>            Assignee: Ivan A. Veselovsky
>            Priority: Minor
>         Attachments: HADOOP-9046-branch-0.23--c.patch, 
> HADOOP-9046-branch-0.23-over-9049.patch, HADOOP-9046-branch-0.23.patch, 
> HADOOP-9046--c.patch, HADOOP-9046-over-9049.patch, HADOOP-9046.patch
>
>
> The class org.apache.hadoop.fs.DelegationTokenRenewer.RenewAction<T> has zero 
> coverage in entire cumulative test run. Provide test(s) to cover this class.
> Note: the request submitted to HDFS project because the class likely to be 
> tested by tests in that project.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to