[
https://issues.apache.org/jira/browse/HADOOP-12973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15230762#comment-15230762
]
Colin Patrick McCabe edited comment on HADOOP-12973 at 4/7/16 6:32 PM:
-----------------------------------------------------------------------
bq. It makes it more obvious when someone overrides the class where things are.
Hmm. How about making the class {{final}} instead?
Re: {{DU}} versus {{WindowsDU}}. If you really want to separate the classes, I
don't object, but I don't want the {{WindowsDU}} to be a subclass of the Linux
{{DU}}. That is just weird.
bq. Shutdown is needed. So it's very strange to have a shutdown without a start.
There is a start-- in {{GetSpaceUsedBuilder}}. Having an "init" method that
you have to call after initialization is an anti-pattern. There is no reason
why the user should have to care whether {{GetSpaceUsedBuilder}} contains a
thread or not-- many implementations won't need a thread. The fact that not
all subclasses need threads is a good sign that thread management doesn't
belong in the common interface.
I'm also curious how you feel about the idea of making the interface
{{Closeable}}, as we've done with many other interfaces such as
{{FailoverProxyProvider}}, {{ServicePlugin}}, {{BlockReader}}, {{Peer}},
{{PeerServer}}, {{FsVolumeReference}}, etc. etc. The compiler and various
linters warn about failures to close {{Closeable}} objects in many cases, but
not about failure to call custom shutdown funtions.
was (Author: cmccabe):
bq. It makes it more obvious when someone overrides the class where things are.
Hmm. How about making the class {{final}} instead?
Re: {{DU}} versus {{WindowsDU}}. If you really want to separate the classes, I
don't object, but I don't want the {{WindowsDU}} to be a subclass of the Linux
{{DU}}. That is just weird.
bq. Shutdown is needed. So it's very strange to have a shutdown without a start.
There is a start-- in {{GetSpaceUsedBuilder}}. Having an "init" method that
you have to call after initialization is an anti-pattern. There is no reason
why the user should have to care whether {{GetSpaceUsedBuilder}} contains a
thread or not-- many implementations won't need a thread. The fact that not
all subclasses need threads is a good sign that thread management doesn't
belong in the common interface.
I'm also curious how you feel about the idea of making the interface
{{Closeable}}, as we've done with many other interfaces such as
{{FailoverProxyProvider}}, {{ServicePlugin}}, {{BlockReader}}, {{Peer}},
{{PeerServer}}, {{FsVolumeReference}}, etc. etc.
> 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-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)
