[
https://issues.apache.org/jira/browse/HDFS-9847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15204945#comment-15204945
]
Chris Douglas edited comment on HDFS-9847 at 3/21/16 8:53 PM:
--------------------------------------------------------------
The current patch changes {{Configuration::getTimeDuration}} to throw when
losing precision:
{noformat}
// Configuration.java
+ long raw = Long.parseLong(timeStr);
+ long converted = unit.convert(raw, vUnit.unit());
+ if (vUnit.unit().convert(converted, unit) != raw) {
+ throw new HadoopIllegalArgumentException("Loss of precision converting "
+ + timeStr + vUnit.suffix() + " to " + unit);
}
- return unit.convert(Long.parseLong(vStr), vUnit.unit());
+ return converted;
// TestConfiguration
Configuration conf = new Configuration(false);
conf.setTimeDuration("test.time.a", 7L, SECONDS);
assertEquals("7s", conf.get("test.time.a"));
- assertEquals(0L, conf.getTimeDuration("test.time.a", 30, MINUTES));
{noformat}
This changes the contract. In the current version, the caller determines the
precision (as detailed in the javadoc). This is correct; the caller knows
reasonable precision, and can perform checks (e.g., unexpected value of 0) in
its context. The {{Configuration}} has no context, and providing the expected
precision as an argument overengineers the interface. If I want to give a
heartbeat interval in microseconds that's my right as a lunatic, but the caller
should not throw because it only cares about the value in seconds.
(aside) Why {{HadoopIllegalArgumentException}} instead of
{{java.lang.IllegalArgumentException}}? Not why is is thrown here, but why does
it exist?
was (Author: chris.douglas):
The current patch changes {{Configuration::getTimeDuration}} to throw when
losing precision:
{{noformat}}
// Configuration.java
+ long raw = Long.parseLong(timeStr);
+ long converted = unit.convert(raw, vUnit.unit());
+ if (vUnit.unit().convert(converted, unit) != raw) {
+ throw new HadoopIllegalArgumentException("Loss of precision converting "
+ + timeStr + vUnit.suffix() + " to " + unit);
}
- return unit.convert(Long.parseLong(vStr), vUnit.unit());
+ return converted;
// TestConfiguration
Configuration conf = new Configuration(false);
conf.setTimeDuration("test.time.a", 7L, SECONDS);
assertEquals("7s", conf.get("test.time.a"));
- assertEquals(0L, conf.getTimeDuration("test.time.a", 30, MINUTES));
{{noformat}}
This changes the contract. In the current version, the caller determines the
precision (as detailed in the javadoc). This is correct; the caller knows
reasonable precision, and can perform checks (e.g., unexpected value of 0) in
its context. The {{Configuration}} has no context, and providing the expected
precision as an argument overengineers the interface. If I want to give a
heartbeat interval in microseconds that's my right as a lunatic, but the caller
should not throw because it only cares about the value in seconds.
(aside) Why {{HadoopIllegalArgumentException}} instead of
{{java.lang.IllegalArgumentException}}? Not why is is thrown here, but why does
it exist?
> HDFS configuration without time unit name should accept friendly time units
> ---------------------------------------------------------------------------
>
> Key: HDFS-9847
> URL: https://issues.apache.org/jira/browse/HDFS-9847
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Affects Versions: 2.7.1
> Reporter: Lin Yiqun
> Assignee: Lin Yiqun
> Attachments: HDFS-9847.001.patch, HDFS-9847.002.patch,
> HDFS-9847.003.patch, HDFS-9847.004.patch, HDFS-9847.005.patch,
> HDFS-9847.006.patch, timeduration-w-y.patch
>
>
> In HDFS-9821, it talks about the issue of leting existing keys use friendly
> units e.g. 60s, 5m, 1d, 6w etc. But there are som configuration key names
> contain time unit name, like {{dfs.blockreport.intervalMsec}}, so we can make
> some other configurations which without time unit name to accept friendly
> time units. The time unit {{seconds}} is frequently used in hdfs. We can
> updating this configurations first.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
