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

Chris Douglas commented on HDFS-9847:
-------------------------------------

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)

Reply via email to