[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634233#comment-16634233 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661448 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) --- End diff -- This is not a very clear javadoc that describe what this method does (it should have the same javadoc as the `DFSUtil.getHaNnRpcAddresses` method). > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634235#comment-16634235 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221660130 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) + * @param config + * @return + */ + private static Set>> getEntries(Configuration config) { +return iDFSUtilClientExists ? invoke(config) : +DFSUtil.getHaNnRpcAddresses(config).entrySet(); + } + + private static Set>> invoke(Configuration config) { --- End diff -- Please name this method with a more appropriate name, rather than a generic name `invoke`. > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634234#comment-16634234 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221660462 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** --- End diff -- Use two asterisks instead of three `/**` > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661663 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -345,6 +388,28 @@ private static FileSystem getFileSystem(LocationFactory locationFactory) throws return null; } + private static void handleLogAction(Exception e) { --- End diff -- This method is unnecessary, better log it in place. E.g. in the static initializer above: ```java static { try { Class dfsUtilsClientClazz = Class.forName("org.apache.hadoop.hdfs.DFSUtilClient"); getHaNnRpcAddressesMethod = dfsUtilsClientClazz.getMethod("getHaNnRpcAddresses", Configuration.class); hasDFSUtilClient = true; } catch (ClassNotFoundException e) { // Expected for Hadoop version < 2.8, hence log it as debug only to no polluting the logs LOG.debug("No DFSUtilClient found", e); } catch (NoSuchMethodException e) { // This is unexpected for not founding the getHaNnRpcAddresses method if the DFSUtilClient class exists LOG.warn("No DFSUtilClient.getHaNnRpcAddresses method found. Getting HA NameNode address might fail.", e); } } ``` ---
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634238#comment-16634238 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661663 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -345,6 +388,28 @@ private static FileSystem getFileSystem(LocationFactory locationFactory) throws return null; } + private static void handleLogAction(Exception e) { --- End diff -- This method is unnecessary, better log it in place. E.g. in the static initializer above: ```java static { try { Class dfsUtilsClientClazz = Class.forName("org.apache.hadoop.hdfs.DFSUtilClient"); getHaNnRpcAddressesMethod = dfsUtilsClientClazz.getMethod("getHaNnRpcAddresses", Configuration.class); hasDFSUtilClient = true; } catch (ClassNotFoundException e) { // Expected for Hadoop version < 2.8, hence log it as debug only to no polluting the logs LOG.debug("No DFSUtilClient found", e); } catch (NoSuchMethodException e) { // This is unexpected for not founding the getHaNnRpcAddresses method if the DFSUtilClient class exists LOG.warn("No DFSUtilClient.getHaNnRpcAddresses method found. Getting HA NameNode address might fail.", e); } } ``` > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661290 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) + * @param config + * @return + */ + private static Set>> getEntries(Configuration config) { +return iDFSUtilClientExists ? invoke(config) : +DFSUtil.getHaNnRpcAddresses(config).entrySet(); + } + + private static Set>> invoke(Configuration config) { +try { + return ((Map) getHaNnRpcAddressesMethod.invoke(null, config)).entrySet(); --- End diff -- Why casting to `Map` while the return type of this method is `Set`?? ---
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661448 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) --- End diff -- This is not a very clear javadoc that describe what this method does (it should have the same javadoc as the `DFSUtil.getHaNnRpcAddresses` method). ---
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634237#comment-16634237 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221663573 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -72,7 +77,23 @@ HADOOP_26 } - private static final Logger LOG = LoggerFactory.getLogger(YarnUtils.class); + private static boolean iDFSUtilClientExists = false; // use this to judge if the hadoop version is above 2.8 --- End diff -- What does the `i` prefix mean? Better just call this `hasDFSUtilClient`. > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634239#comment-16634239 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221660324 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) + * @param config + * @return + */ + private static Set>> getEntries(Configuration config) { --- End diff -- Please name this method with a more specific name, such as `getHaNnRpcAddress`. > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TWILL-262) YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is removed from DFSUtils from hadoop-2.8
[ https://issues.apache.org/jira/browse/TWILL-262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16634236#comment-16634236 ] ASF GitHub Bot commented on TWILL-262: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221661290 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) + * @param config + * @return + */ + private static Set>> getEntries(Configuration config) { +return iDFSUtilClientExists ? invoke(config) : +DFSUtil.getHaNnRpcAddresses(config).entrySet(); + } + + private static Set>> invoke(Configuration config) { +try { + return ((Map) getHaNnRpcAddressesMethod.invoke(null, config)).entrySet(); --- End diff -- Why casting to `Map` while the return type of this method is `Set`?? > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > > > Key: TWILL-262 > URL: https://issues.apache.org/jira/browse/TWILL-262 > Project: Apache Twill > Issue Type: Bug > Components: yarn >Affects Versions: 0.8.0, 0.9.0, 0.11.0, 0.12.0, 0.13.0 >Reporter: Hongyuan Li >Priority: Major > Attachments: errors.txt > > > YarnUtils#cloneHaNnCredentials uses DFSUtil#getHaNnRpcAddresses, which is > removed from DFSUtils from hadoop-2.8 > current code > {code} > public static void cloneHaNnCredentials(Configuration config) throws > IOException { > …… > // Loop through all name services. Each name service could have multiple > name node associated with it. > for (Map.Entry> entry : > DFSUtil.getHaNnRpcAddresses(config).entrySet()) { > String nsId = entry.getKey(); > Map addressesInNN = entry.getValue(); > if (!HAUtil.isHAEnabled(config, nsId) || addressesInNN == null || > addressesInNN.isEmpty()) { > continue; > } > …… > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221660462 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** --- End diff -- Use two asterisks instead of three `/**` ---
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221663573 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -72,7 +77,23 @@ HADOOP_26 } - private static final Logger LOG = LoggerFactory.getLogger(YarnUtils.class); + private static boolean iDFSUtilClientExists = false; // use this to judge if the hadoop version is above 2.8 --- End diff -- What does the `i` prefix mean? Better just call this `hasDFSUtilClient`. ---
[GitHub] twill pull request #71: TWILL-262 YarnUtils#cloneHaNnCredentials uses DFSUti...
Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/71#discussion_r221660130 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java --- @@ -202,6 +223,28 @@ public static void cloneHaNnCredentials(Configuration config) throws IOException } } + /*** + * When hadoop_version > 2.8.0, class DFSUtils has no method getHaNnRpcAddresses(Configuration config) + * @param config + * @return + */ + private static Set>> getEntries(Configuration config) { +return iDFSUtilClientExists ? invoke(config) : +DFSUtil.getHaNnRpcAddresses(config).entrySet(); + } + + private static Set>> invoke(Configuration config) { --- End diff -- Please name this method with a more appropriate name, rather than a generic name `invoke`. ---