You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-dev@hadoop.apache.org by Jay Vyas <ja...@gmail.com> on 2013/04/19 18:10:13 UTC

Convention question for using DFSConfigKey constants : are zeros magic?

I recently looked into the HDFS source tree to determine idioms with
respect to a hairy debate about the threshold between what is and is not a
magic number, and found that :  And it appears that the number zero is NOT
considered magic - at least not in the HDFS source code.

I found that that DFSConfigKeys.java defines DEFAULT values of zeros for
some fields, and those defaults result in non-quantitative interpretation
of the field.

For example:
dfs.image.transfer.bandwidthPerSec

Is commented like so:
public static final long DFS_IMAGE_TRANSFER_RATE_DEFAULT = 0;  //no
throttling

However in the implementation of these defaults, "magic" zeros are used
without commenting:
if (transferBandwidth > 0) {
      throttler = new DataTransferThrottler(transferBandwidth);
}

--------

Seems like the 0 above would be better replaced with
DFS_IMAGE_TRANSFER_RATE_DEFAULT since the "no throttling" behaviour is
defined with the constant in the DFSConfigKeys file, and not defined in the
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java.


--------

Trying to get a feel for if there are conventions  to enforce in some code
reviews for our hadoop dependent configuration code.  We'd like to follow
hadoopy idioms if possible..


-- 
Jay Vyas
http://jayunit100.blogspot.com

Re: Convention question for using DFSConfigKey constants : are zeros magic?

Posted by "Aaron T. Myers" <at...@cloudera.com>.
On Fri, Apr 19, 2013 at 3:40 PM, Jay Vyas <ja...@gmail.com> wrote:

> The more generic question is wether or not there is enforcement of naming
> conventions or commenting for special values in numeric configuration
> parameters: I'm interpretting your answer as "No"...?
>

There's certainly no "enforcement" of it, no. We try to use consistent
prefixes, append units like "millis" or "seconds" to config settings which
represent time, etc. but this is just by convention.

--
Aaron T. Myers
Software Engineer, Cloudera

Re: Convention question for using DFSConfigKey constants : are zeros magic?

Posted by Jay Vyas <ja...@gmail.com>.
Thanks ! okay that helps to clarify things.

Okay, so the value in referencing the static comment is that it is
commented in the DFS_KEYS file, and declared as 0.  Having the ==0 in code
defines this default behaviour implicitly, so a change to the code would
make that code inconsistent with the comment in the DFS Keys file.  Having
run into some tricky configuration changes in the past it concerns me a
little... but...

The more generic question is wether or not there is enforcement of naming
conventions or commenting for special values in numeric configuration
parameters: I'm interpretting your answer as "No"...?

Thats fine.. but  just clarifying :) ?

On Fri, Apr 19, 2013 at 2:23 PM, Aaron T. Myers <at...@cloudera.com> wrote:

> Hi Jay,
>
> On Sat, Apr 20, 2013 at 1:10 AM, Jay Vyas <ja...@gmail.com> wrote:
>
> > I recently looked into the HDFS source tree to determine idioms with
> > respect to a hairy debate about the threshold between what is and is not
> a
> > magic number, and found that :  And it appears that the number zero is
> NOT
> > considered magic - at least not in the HDFS source code.
> >
>
> It's certainly not magic in the Configuration class interpretation of it,
> though I think if you surveyed the full source code you'd find that there
> won't be much consistency with regard to ad hoc checks in the code for
> certain values, like you've identified below.
>
>
> >
> > I found that that DFSConfigKeys.java defines DEFAULT values of zeros for
> > some fields, and those defaults result in non-quantitative interpretation
> > of the field.
> >
> > For example:
> > dfs.image.transfer.bandwidthPerSec
> >
> > Is commented like so:
> > public static final long DFS_IMAGE_TRANSFER_RATE_DEFAULT = 0;  //no
> > throttling
> >
> > However in the implementation of these defaults, "magic" zeros are used
> > without commenting:
> > if (transferBandwidth > 0) {
> >       throttler = new DataTransferThrottler(transferBandwidth);
> > }
> >
> > --------
> >
> > Seems like the 0 above would be better replaced with
> > DFS_IMAGE_TRANSFER_RATE_DEFAULT since the "no throttling" behaviour is
> > defined with the constant in the DFSConfigKeys file, and not defined in
> the
> >
> >
> hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java.
> >
>
> I don't agree with this. What if we later changed the default to something
> greater than 0 in DFSConfigKeys? If the code were comparing against the
> value DFS_IMAGE_TRANSFER_RATE_DEFAULT, the check in the code would then be
> wrong. The only value for that config that should denote "no throttling" is
> 0, regardless of what the default is, so the explicit comparison against 0
> makes sense to me.
>
>
> >
> >
> > --------
> >
> > Trying to get a feel for if there are conventions  to enforce in some
> code
> > reviews for our hadoop dependent configuration code.  We'd like to follow
> > hadoopy idioms if possible..
> >
>
> I'd say the main conventions you should concern yourself with for this
> purpose is config setting naming, e.g. use consistent prefixes within your
> own code, use lower case separated by dots.and-dashes, etc.
>
>
> >
> >
> > --
> > Jay Vyas
> > http://jayunit100.blogspot.com
> >
>



-- 
Jay Vyas
http://jayunit100.blogspot.com

Re: Convention question for using DFSConfigKey constants : are zeros magic?

Posted by "Aaron T. Myers" <at...@cloudera.com>.
Hi Jay,

On Sat, Apr 20, 2013 at 1:10 AM, Jay Vyas <ja...@gmail.com> wrote:

> I recently looked into the HDFS source tree to determine idioms with
> respect to a hairy debate about the threshold between what is and is not a
> magic number, and found that :  And it appears that the number zero is NOT
> considered magic - at least not in the HDFS source code.
>

It's certainly not magic in the Configuration class interpretation of it,
though I think if you surveyed the full source code you'd find that there
won't be much consistency with regard to ad hoc checks in the code for
certain values, like you've identified below.


>
> I found that that DFSConfigKeys.java defines DEFAULT values of zeros for
> some fields, and those defaults result in non-quantitative interpretation
> of the field.
>
> For example:
> dfs.image.transfer.bandwidthPerSec
>
> Is commented like so:
> public static final long DFS_IMAGE_TRANSFER_RATE_DEFAULT = 0;  //no
> throttling
>
> However in the implementation of these defaults, "magic" zeros are used
> without commenting:
> if (transferBandwidth > 0) {
>       throttler = new DataTransferThrottler(transferBandwidth);
> }
>
> --------
>
> Seems like the 0 above would be better replaced with
> DFS_IMAGE_TRANSFER_RATE_DEFAULT since the "no throttling" behaviour is
> defined with the constant in the DFSConfigKeys file, and not defined in the
>
> hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java.
>

I don't agree with this. What if we later changed the default to something
greater than 0 in DFSConfigKeys? If the code were comparing against the
value DFS_IMAGE_TRANSFER_RATE_DEFAULT, the check in the code would then be
wrong. The only value for that config that should denote "no throttling" is
0, regardless of what the default is, so the explicit comparison against 0
makes sense to me.


>
>
> --------
>
> Trying to get a feel for if there are conventions  to enforce in some code
> reviews for our hadoop dependent configuration code.  We'd like to follow
> hadoopy idioms if possible..
>

I'd say the main conventions you should concern yourself with for this
purpose is config setting naming, e.g. use consistent prefixes within your
own code, use lower case separated by dots.and-dashes, etc.


>
>
> --
> Jay Vyas
> http://jayunit100.blogspot.com
>