K0K0V0K commented on code in PR #7813:
URL: https://github.com/apache/hadoop/pull/7813#discussion_r2218613928
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java:
##########
@@ -107,17 +107,29 @@ public static void
setNMWebAppHostNameAndPort(Configuration conf,
*/
public static <T, R> R execOnActiveRM(Configuration conf,
ThrowingBiFunction<String, T, R> func, T arg) throws Exception {
- int haIndex = 0;
+ int activeRMIndex = 0;
+ int rmCount = 1;
+
if (HAUtil.isHAEnabled(conf)) {
Review Comment:
What if we try some early return pattern?
```
if (!HAUtil.isHAEnabled(conf)) {
// If i understand right here we have just 1 RM so we can just try with
that
String rmAddress = getRMWebAppURLWithScheme(conf, 0);
return func.apply(rmAddress, arg);
}
// try RMHAUtils.findActiveRMHAId(conf)
// if null do the iteration
// finally apply and return
```
Maybe will be a bit easyer to read cause now we have the
```
int activeRMIndex = 0;
int rmCount = 1;
```
code what related to non HA, than HA code, than some mix.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java:
##########
@@ -107,17 +107,29 @@ public static void
setNMWebAppHostNameAndPort(Configuration conf,
*/
public static <T, R> R execOnActiveRM(Configuration conf,
ThrowingBiFunction<String, T, R> func, T arg) throws Exception {
- int haIndex = 0;
+ int activeRMIndex = 0;
+ int rmCount = 1;
+
if (HAUtil.isHAEnabled(conf)) {
+ ArrayList<String> rmIds = (ArrayList<String>) HAUtil.getRMHAIds(conf);
+ rmCount = rmIds.size();
String activeRMId = RMHAUtils.findActiveRMHAId(conf);
if (activeRMId != null) {
- haIndex = new ArrayList<>(HAUtil.getRMHAIds(conf)).indexOf(activeRMId);
- } else {
- throw new ConnectException("No Active RM available");
+ activeRMIndex = rmIds.indexOf(activeRMId);
+ }
+ }
+
+ // In HA mode activeRMId can be fetched only if user have permission to
check service states.
+ // Otherwise, we find the active one by iterating through the RMs
+ for (int i = activeRMIndex; i < rmCount; i++) {
+ try {
+ String rmAddress = getRMWebAppURLWithScheme(conf, i);
+ return func.apply(rmAddress, arg);
+ } catch (Exception e) {
+ // Ignore and try next RM if there are any
Review Comment:
Maybe some trace level LOG can be use full
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java:
##########
@@ -107,17 +107,29 @@ public static void
setNMWebAppHostNameAndPort(Configuration conf,
*/
public static <T, R> R execOnActiveRM(Configuration conf,
ThrowingBiFunction<String, T, R> func, T arg) throws Exception {
- int haIndex = 0;
+ int activeRMIndex = 0;
+ int rmCount = 1;
+
if (HAUtil.isHAEnabled(conf)) {
+ ArrayList<String> rmIds = (ArrayList<String>) HAUtil.getRMHAIds(conf);
Review Comment:
Can we cast to simple List interface here? Maybe will be a bit more robust.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
