Repository: impala Updated Branches: refs/heads/master b27537a15 -> 5842c8406
IMPALA-6384: RequestPoolService should honor custom group mapping config Due to the way in which we instantiate fair scheduler allocation loader, we donot read the config overrides from the HDFS config files. This is an unexpected behavior from users' POV since we typically support overrides like custom user -> group mapping via HDFS config (for ex: LDAPGroupsMapping) that eventually affects the query -> pool assignment. Fix: This patch loads the hadoop default configuration so that the underlying QueuePlacementPolicy is based on user specified overrides. Testing (manual): Changed the core-site.xml to use LDAPGroupsMapping instead of the default ShellBasedUnixGroupsMapping and confirmed that the correct group mapping plugin is loaded, by adding additional logging. Also, modified TestRequestPoolService to assert that the core-site xml overrides are loaded. Change-Id: Ibb93870c0cc37e2432a643a274931f1d3d13fb96 Reviewed-on: http://gerrit.cloudera.org:8080/9000 Reviewed-by: Bharath Vissapragada <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/20daa4d5 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/20daa4d5 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/20daa4d5 Branch: refs/heads/master Commit: 20daa4d516987242ed7da5fe9474267666cf8add Parents: b27537a Author: Bharath Vissapragada <[email protected]> Authored: Wed Jan 10 13:40:34 2018 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Jan 11 22:52:29 2018 +0000 ---------------------------------------------------------------------- .../scheduler/fair/QueuePlacementPolicy.java | 6 +++++- .../org/apache/impala/util/RequestPoolService.java | 15 ++++++++++++++- .../apache/impala/util/TestRequestPoolService.java | 6 ++++++ .../common/etc/hadoop/conf/core-site.xml.tmpl | 7 +++++++ 4 files changed, 32 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/20daa4d5/common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java ---------------------------------------------------------------------- diff --git a/common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java b/common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java index 74c690c..a63dd0f 100644 --- a/common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java +++ b/common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/QueuePlacementPolicy.java @@ -54,6 +54,7 @@ public class QueuePlacementPolicy { ruleClasses = Collections.unmodifiableMap(map); } + private final Configuration conf_; // Testing only. private final List<QueuePlacementRule> rules; private final Map<FSQueueType, Set<String>> configuredQueues; private final Groups groups; @@ -73,6 +74,7 @@ public class QueuePlacementPolicy { } this.rules = rules; this.configuredQueues = configuredQueues; + this.conf_ = conf; groups = new Groups(conf); } @@ -178,4 +180,6 @@ public class QueuePlacementPolicy { public List<QueuePlacementRule> getRules() { return rules; } -} \ No newline at end of file + + public Configuration getConf() { return conf_; } +} http://git-wip-us.apache.org/repos/asf/impala/blob/20daa4d5/fe/src/main/java/org/apache/impala/util/RequestPoolService.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/util/RequestPoolService.java b/fe/src/main/java/org/apache/impala/util/RequestPoolService.java index 5cea0e1..a104399 100644 --- a/fe/src/main/java/org/apache/impala/util/RequestPoolService.java +++ b/fe/src/main/java/org/apache/impala/util/RequestPoolService.java @@ -39,6 +39,7 @@ import org.apache.impala.common.ByteUnits; import org.apache.impala.common.ImpalaException; import org.apache.impala.common.InternalException; import org.apache.impala.common.JniUtil; +import org.apache.impala.common.RuntimeEnv; import org.apache.impala.thrift.TErrorCode; import org.apache.impala.thrift.TPoolConfigParams; import org.apache.impala.thrift.TPoolConfig; @@ -170,7 +171,9 @@ public class RequestPoolService { throw new IllegalArgumentException( "Unable to find allocation configuration file: " + fsAllocationPath); } - Configuration allocConf = new Configuration(false); + // Load the default Hadoop configuration files for picking up overrides like custom + // group mapping plugins etc. + Configuration allocConf = new Configuration(); allocConf.set(FairSchedulerConfiguration.ALLOCATION_FILE, fsAllocationURL.getPath()); allocLoader_ = new AllocationFileLoaderService(); allocLoader_.init(allocConf); @@ -450,4 +453,14 @@ public class RequestPoolService { UserGroupInformation ugi = UserGroupInformation.createRemoteUser(shortName); return allocationConf_.get().hasAccess(pool, QueueACL.SUBMIT_APPLICATIONS, ugi); } + + /** + * Returns the AllocationConfiguration corresponding to this instance of + * RequestPoolService. + */ + @VisibleForTesting + AllocationConfiguration getAllocationConfig() { + Preconditions.checkState(RuntimeEnv.INSTANCE.isTestEnv()); + return allocationConf_.get(); + } } http://git-wip-us.apache.org/repos/asf/impala/blob/20daa4d5/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java b/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java index fc734ab..622c7a9 100644 --- a/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java +++ b/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java @@ -45,6 +45,7 @@ import org.apache.impala.thrift.TErrorCode; import org.apache.impala.thrift.TPoolConfig; import org.apache.impala.thrift.TResolveRequestPoolParams; import org.apache.impala.thrift.TResolveRequestPoolResult; +import org.apache.impala.yarn.server.resourcemanager.scheduler.fair.QueuePlacementPolicy; import com.google.common.collect.Iterables; import com.google.common.io.Files; @@ -116,6 +117,11 @@ public class TestRequestPoolService { poolService_.llamaConfWatcher_.setCheckIntervalMs(CHECK_INTERVAL_MS); } poolService_.start(); + // Make sure that the Hadoop configuration from classpath is used for the underlying + // QueuePlacementPolicy. + QueuePlacementPolicy policy = poolService_.getAllocationConfig().getPlacementPolicy(); + Configuration conf = policy.getConf(); + Assert.assertTrue(conf.getBoolean("impala.core-site.overridden", false)); } @BeforeClass http://git-wip-us.apache.org/repos/asf/impala/blob/20daa4d5/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl ---------------------------------------------------------------------- diff --git a/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl b/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl index 1f53e97..2b53d82 100644 --- a/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl +++ b/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl @@ -110,4 +110,11 @@ DEFAULT</value> <value>https://login.windows.net/${azure_tenant_id}/oauth2/token</value> </property> + <!-- This property can be used in tests to ascertain that this core-site.xml from + the classpath has been loaded. (Ex: TestRequestPoolService) --> + <property> + <name>impala.core-site.overridden</name> + <value>true</value> + </property> + </configuration>
