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

Colin Patrick McCabe commented on HADOOP-12973:
-----------------------------------------------

Thanks, [~eclark].  Looks good!  The Windows code looks cleaner than before.

{code}
    try {
      dfsUsage.close();
    } catch (IOException ioe){
      LOG.warn("Error trying to shutdown GetUsedSpace background thread", ioe);
    }
{code}
Can use {{IOUtils#cleanup}} here?

I'm wonder if we could have GetSpaceUsed just be an interface with only one 
method... {{long getSpace()}} or something like that.  A method that just 
synchronously retrieves the amount of space used, blocking for as long as it 
takes.

Then, we could have another class which does all this thread management and 
value caching stuff.  It seems unrelated to the {{GetSpaceUsed}} interface.  
Like if I'm implementing {{JNIGetSpaceUsed}}, I don't care about thread 
management.  I just want to implement the method which gets the amount of space 
used, and leave the thread management the same.  I think that's the direction 
you were going with the {{GetSpaceUsed}} base class, but feels messy to make 
the implementation classes reach back up into the base class and play with 
atomic variables.

> make DU pluggable
> -----------------
>
>                 Key: HADOOP-12973
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12973
>             Project: Hadoop Common
>          Issue Type: Sub-task
>            Reporter: Elliott Clark
>            Assignee: Elliott Clark
>         Attachments: HADOOP-12973v0.patch, HADOOP-12973v1.patch, 
> HADOOP-12973v10.patch, HADOOP-12973v11.patch, HADOOP-12973v2.patch, 
> HADOOP-12973v3.patch, HADOOP-12973v5.patch, HADOOP-12973v6.patch, 
> HADOOP-12973v7.patch, HADOOP-12973v8.patch, HADOOP-12973v9.patch
>
>
> If people are concerned about replacing the call to DU. Then an easy first 
> step is to make it pluggable. Then it's possible to replace it with something 
> while leaving the default alone.



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

Reply via email to