You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2020/02/25 17:09:43 UTC
[impala] 02/04: IMPALA-8852: Skip short-circuit config check for
dedicated coordinator
This is an automated email from the ASF dual-hosted git repository.
tmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 777d0d203f138183d65885b523b619421b487714
Author: Tamas Mate <tm...@cloudera.com>
AuthorDate: Thu Feb 6 14:21:10 2020 +0100
IMPALA-8852: Skip short-circuit config check for dedicated coordinator
ImpalaD should not abort when running as dedicated coodinator and DataNode is
not available on the host. This change adds a condition to skip the short-
circuit socket path directory checks when ImpalaD is started with
'is_executor=false' flag.
Testing:
- Added test to JniFrontendTest.java to verify the short-circuit directory
check is skipped if ImpalaD is started as dedicated coordinator mode.
- Manually tested the appearance of the warning message with:
start-impala-cluster.py --num_coordinators 1 --use_exclusive_coordinators true
Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5
Reviewed-on: http://gerrit.cloudera.org:8080/15173
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/util/backend-gflag-util.cc | 2 +
common/thrift/BackendGflags.thrift | 22 ++++---
.../org/apache/impala/service/BackendConfig.java | 4 ++
.../org/apache/impala/service/JniFrontend.java | 8 ++-
.../org/apache/impala/service/JniFrontendTest.java | 73 +++++++++++++++++++++-
5 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/be/src/util/backend-gflag-util.cc b/be/src/util/backend-gflag-util.cc
index d04e344..f3e3755 100644
--- a/be/src/util/backend-gflag-util.cc
+++ b/be/src/util/backend-gflag-util.cc
@@ -77,6 +77,7 @@ DECLARE_bool(recursively_list_partitions);
DECLARE_string(query_event_hook_classes);
DECLARE_int32(query_event_hook_nthreads);
DECLARE_bool(is_executor);
+DECLARE_bool(is_coordinator);
DECLARE_bool(use_dedicated_coordinator_estimates);
DECLARE_string(blacklisted_dbs);
DECLARE_bool(unlock_zorder_sort);
@@ -165,6 +166,7 @@ Status GetThriftBackendGflags(JNIEnv* jni_env, jbyteArray* cfg_bytes) {
cfg.__set_query_event_hook_classes(FLAGS_query_event_hook_classes);
cfg.__set_query_event_hook_nthreads(FLAGS_query_event_hook_nthreads);
cfg.__set_is_executor(FLAGS_is_executor);
+ cfg.__set_is_coordinator(FLAGS_is_coordinator);
cfg.__set_use_dedicated_coordinator_estimates(
FLAGS_use_dedicated_coordinator_estimates);
cfg.__set_blacklisted_dbs(FLAGS_blacklisted_dbs);
diff --git a/common/thrift/BackendGflags.thrift b/common/thrift/BackendGflags.thrift
index 2498ecd..90fad1a 100644
--- a/common/thrift/BackendGflags.thrift
+++ b/common/thrift/BackendGflags.thrift
@@ -134,23 +134,25 @@ struct TBackendGflags {
55: required bool is_executor
- 56: required bool use_dedicated_coordinator_estimates
+ 56: required bool is_coordinator
- 57: required string blacklisted_dbs
+ 57: required bool use_dedicated_coordinator_estimates
- 58: required string blacklisted_tables
+ 58: required string blacklisted_dbs
- 59: required bool unlock_zorder_sort
+ 59: required string blacklisted_tables
- 60: required string min_privilege_set_for_show_stmts
+ 60: required bool unlock_zorder_sort
- 61: required bool mt_dop_auto_fallback
+ 61: required string min_privilege_set_for_show_stmts
- 62: required i32 num_expected_executors
+ 62: required bool mt_dop_auto_fallback
- 63: required i32 num_check_authorization_threads
+ 63: required i32 num_expected_executors
- 64: required bool use_customized_user_groups_mapper_for_ranger
+ 64: required i32 num_check_authorization_threads
- 65: required bool enable_column_masking
+ 65: required bool use_customized_user_groups_mapper_for_ranger
+
+ 66: required bool enable_column_masking
}
diff --git a/fe/src/main/java/org/apache/impala/service/BackendConfig.java b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
index ac7f66b..c3957b9 100644
--- a/fe/src/main/java/org/apache/impala/service/BackendConfig.java
+++ b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
@@ -230,4 +230,8 @@ public class BackendConfig {
KerberosName.setRules(defaultRule);
}
}
+
+ public boolean isDedicatedCoordinator() {
+ return (backendCfg_.is_executor == false) && (backendCfg_.is_coordinator == true);
+ }
}
diff --git a/fe/src/main/java/org/apache/impala/service/JniFrontend.java b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
index f550956..2d47268 100644
--- a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
+++ b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
@@ -718,7 +718,8 @@ public class JniFrontend {
* Returns an error message if short circuit reads are enabled but misconfigured.
* Otherwise, returns an empty string,
*/
- private String checkShortCircuitRead(Configuration conf) {
+ @VisibleForTesting
+ protected static String checkShortCircuitRead(Configuration conf) {
if (!conf.getBoolean(DFSConfigKeys.DFS_CLIENT_READ_SHORTCIRCUIT_KEY,
DFSConfigKeys.DFS_CLIENT_READ_SHORTCIRCUIT_DEFAULT)) {
LOG.info("Short-circuit reads are not enabled.");
@@ -741,7 +742,10 @@ public class JniFrontend {
// The socket path parent directory must be readable and executable.
File socketFile = new File(domainSocketPath);
File socketDir = socketFile.getParentFile();
- if (socketDir == null || !socketDir.canRead() || !socketDir.canExecute()) {
+ if (BackendConfig.INSTANCE.isDedicatedCoordinator()) {
+ LOG.warn("Dedicated coordinator instance will not read local data via "
+ + "short-circuit reads, socket path directory checks are skipped.");
+ } else if (socketDir == null || !socketDir.canRead() || !socketDir.canExecute()) {
errorCause.append(prefix);
errorCause.append("Impala cannot read or execute the parent directory of ");
errorCause.append(DFSConfigKeys.DFS_DOMAIN_SOCKET_PATH_KEY);
diff --git a/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java b/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
index b4f8dd8..26752d9 100644
--- a/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
+++ b/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
@@ -17,19 +17,45 @@
package org.apache.impala.service;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.util.Random;
+
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.hdfs.DFSConfigKeys;
import org.apache.hadoop.security.JniBasedUnixGroupsMappingWithFallback;
import org.apache.hadoop.security.JniBasedUnixGroupsNetgroupMappingWithFallback;
import org.apache.hadoop.security.ShellBasedUnixGroupsMapping;
import org.apache.hadoop.security.ShellBasedUnixGroupsNetgroupMapping;
import org.apache.impala.common.ImpalaException;
+import org.apache.impala.thrift.TBackendGflags;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
import org.junit.Test;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
public class JniFrontendTest {
+ private static TBackendGflags origFlags;
+
+ @BeforeClass
+ public static void setup() {
+ // The original BackendConfig need to be mocked, we are saving the values here, so
+ // they can be restored and not break other tests
+ if (BackendConfig.INSTANCE == null) {
+ BackendConfig.create(new TBackendGflags());
+ }
+ origFlags = BackendConfig.INSTANCE.getBackendCfg();
+ }
+
+ @AfterClass
+ public static void teardown() {
+ BackendConfig.create(origFlags);
+ }
+
@Test
public void testCheckGroupsMappingProvider() throws ImpalaException {
Configuration conf = new Configuration();
@@ -55,4 +81,45 @@ public class JniFrontendTest {
ShellBasedUnixGroupsNetgroupMapping.class.getName(),
JniBasedUnixGroupsNetgroupMappingWithFallback.class.getName()));
}
+
+ /**
+ * This test verifies whether short-circuit socket path parent directory configurations
+ * are skipped for coordinator-only mode instances and checked for executors. A socket
+ * directory is created in order to mock the file access.
+ */
+ @Test
+ public void testCheckShortCircuitConfigs() {
+ String tmpDir = System.getProperty("java.io.tmpdir", "/tmp");
+ File socketDir = new File(tmpDir + "/socketTest",
+ "socks." + (System.currentTimeMillis() + "." + (new Random().nextInt())));
+ socketDir.mkdirs();
+ socketDir.getParentFile().setExecutable(false);
+
+ Configuration conf = mock(Configuration.class);
+ when(conf.getBoolean(DFSConfigKeys.DFS_CLIENT_READ_SHORTCIRCUIT_KEY,
+ DFSConfigKeys.DFS_CLIENT_READ_SHORTCIRCUIT_DEFAULT)).thenReturn(true);
+ when(conf.getTrimmed(DFSConfigKeys.DFS_DOMAIN_SOCKET_PATH_KEY,
+ DFSConfigKeys.DFS_DOMAIN_SOCKET_PATH_DEFAULT))
+ .thenReturn(socketDir.getAbsolutePath());
+ when(conf.getBoolean(DFSConfigKeys.DFS_CLIENT_USE_LEGACY_BLOCKREADERLOCAL,
+ DFSConfigKeys.DFS_CLIENT_USE_LEGACY_BLOCKREADERLOCAL_DEFAULT)).thenReturn(false);
+ BackendConfig.INSTANCE = mock(BackendConfig.class);
+
+ when(BackendConfig.INSTANCE.isDedicatedCoordinator()).thenReturn(true);
+ String actualErrorMessage = JniFrontend.checkShortCircuitRead(conf);
+ assertEquals("", actualErrorMessage);
+
+ when(BackendConfig.INSTANCE.isDedicatedCoordinator()).thenReturn(false);
+ actualErrorMessage = JniFrontend.checkShortCircuitRead(conf);
+ assertEquals("Invalid short-circuit reads configuration:\n"
+ + " - Impala cannot read or execute the parent directory of "
+ + DFSConfigKeys.DFS_DOMAIN_SOCKET_PATH_KEY + "\n",
+ actualErrorMessage);
+
+ if (socketDir != null) {
+ socketDir.getParentFile().setExecutable(true);
+ socketDir.delete();
+ socketDir.getParentFile().delete();
+ }
+ }
}
\ No newline at end of file