HADOOP-15523. Shell command timeout given is in seconds whereas it is taken as millisec while scheduling. Contributed by Bilwa S T.

(cherry picked from commit 3905fdb793)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java

@@ -554,7 +554,7 @@ public class CommonConfigurationKeysPublic {
    * <a href="{@docRoot}/../hadoop-project-dist/hadoop-common/core-default.xml">
    * core-default.xml</a>
    */
-  public static final String HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS =
+  public static final String HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_KEY =
       "hadoop.security.groups.shell.command.timeout";
   /**
    * @see
@@ -562,7 +562,7 @@ public class CommonConfigurationKeysPublic {
    * core-default.xml</a>
    */
   public static final long
-          HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS_DEFAULT =
+          HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT =
           0L;
   /**
    * @see

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java

@@ -18,7 +18,6 @@
 package org.apache.hadoop.security;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.StringTokenizer;
@@ -52,7 +51,8 @@ public class ShellBasedUnixGroupsMapping extends Configured
   protected static final Logger LOG =
       LoggerFactory.getLogger(ShellBasedUnixGroupsMapping.class);
 
-  private long timeout = 0L;
+  private long timeout = CommonConfigurationKeys.
+      HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT;
   private static final List<String> EMPTY_GROUPS = new LinkedList<>();
 
   @Override
@@ -61,10 +61,10 @@ public class ShellBasedUnixGroupsMapping extends Configured
     if (conf != null) {
       timeout = conf.getTimeDuration(
           CommonConfigurationKeys.
-              HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS,
+              HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_KEY,
           CommonConfigurationKeys.
-              HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS_DEFAULT,
+              HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT,
-          TimeUnit.SECONDS);
+          TimeUnit.MILLISECONDS);
     }
   }
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java

@@ -1194,7 +1194,7 @@ public abstract class Shell {
 
     /**
      * Returns the timeout value set for the executor's sub-commands.
-     * @return The timeout value in seconds
+     * @return The timeout value in milliseconds
      */
     @VisibleForTesting
     public long getTimeoutInterval() {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedUnixGroupsMapping.java

@@ -173,6 +173,37 @@ public class TestShellBasedUnixGroupsMapping {
     assertTrue(groups.contains("zzz"));
   }
 
+  public long getTimeoutInterval(String timeout) {
+    Configuration conf = new Configuration();
+    String userName = "foobarnonexistinguser";
+    conf.set(
+        CommonConfigurationKeys.HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_KEY,
+        timeout);
+    TestDelayedGroupCommand mapping = ReflectionUtils
+        .newInstance(TestDelayedGroupCommand.class, conf);
+    ShellCommandExecutor executor = mapping.createGroupExecutor(userName);
+    return executor.getTimeoutInterval();
+  }
+
+  @Test
+  public void testShellTimeOutConf() {
+
+    // Test a 1 second max-runtime timeout
+    assertEquals(
+        "Expected the group names executor to carry the configured timeout",
+        1000L, getTimeoutInterval("1s"));
+
+    // Test a 1 minute max-runtime timeout
+    assertEquals(
+        "Expected the group names executor to carry the configured timeout",
+        60000L, getTimeoutInterval("1m"));
+
+    // Test a 1 millisecond max-runtime timeout
+    assertEquals(
+        "Expected the group names executor to carry the configured timeout",
+        1L, getTimeoutInterval("1"));
+  }
+
   private class TestGroupResolvable
       extends ShellBasedUnixGroupsMapping {
     /**
@@ -222,7 +253,7 @@ public class TestShellBasedUnixGroupsMapping {
   private static class TestDelayedGroupCommand
       extends ShellBasedUnixGroupsMapping {
 
-    private Long timeoutSecs = 2L;
+    private Long timeoutSecs = 1L;
 
     TestDelayedGroupCommand() {
       super();
@@ -249,12 +280,12 @@ public class TestShellBasedUnixGroupsMapping {
     String userName = "foobarnonexistinguser";
     String commandTimeoutMessage =
         "ran longer than the configured timeout limit";
-    long testTimeout = 1L;
+    long testTimeout = 500L;
 
     // Test a 1 second max-runtime timeout
     conf.setLong(
         CommonConfigurationKeys.
-            HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS,
+            HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_KEY,
         testTimeout);
 
     TestDelayedGroupCommand mapping =
@@ -306,7 +337,7 @@ public class TestShellBasedUnixGroupsMapping {
     conf = new Configuration();
     long defaultTimeout =
         CommonConfigurationKeys.
-            HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_SECS_DEFAULT;
+            HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT;
 
     mapping =
         ReflectionUtils.newInstance(TestDelayedGroupCommand.class, conf);