You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2020/04/28 15:52:05 UTC
[impala] branch master updated (afe765e -> f4258b5)
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from afe765e Don't filter maven messages about banned dependencies
new 75a6d7b IMPALA-9097: Don't require minicluster for backend tests
new f4258b5 IMPALA-9701: fix data race in BTS
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/runtime/buffered-tuple-stream.cc | 4 +++-
be/src/service/frontend.cc | 8 ++++++--
.../java/org/apache/impala/service/Frontend.java | 23 ++++++++++++++--------
.../org/apache/impala/service/JniFrontend.java | 5 +++--
4 files changed, 27 insertions(+), 13 deletions(-)
[impala] 01/02: IMPALA-9097: Don't require minicluster for backend
tests
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 75a6d7b2bba66825efb3a37f14c9447e64ea584f
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Fri Nov 22 17:58:16 2019 -0800
IMPALA-9097: Don't require minicluster for backend tests
Currently, many backend tests require a running minicluster,
because they initialize a Frontend object that requires a
connection to the Hive Metastore. If the minicluster is not
running or if cluster configurations are missing (i.e.
bin/create-test-configurations.sh needs to run), the backend
tests will fail. The docker based tests always hit this,
because they run the backend tests without a minicluster.
The HMS dependency comes from the Frontend's MetaStoreClientPool,
which is unnecesary for backend tests. This modifies the
code so that it does not initialize this for backend tests,
and thus backend tests pass without a running minicluster.
Testing:
- Ran backend tests without a running minicluster
Change-Id: I8f1b1385853fb23df28d24d38761237e6e5c97a7
Reviewed-on: http://gerrit.cloudera.org:8080/15641
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/service/frontend.cc | 8 ++++++--
.../java/org/apache/impala/service/Frontend.java | 23 ++++++++++++++--------
.../org/apache/impala/service/JniFrontend.java | 5 +++--
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/be/src/service/frontend.cc b/be/src/service/frontend.cc
index baf1089..346ff0c 100644
--- a/be/src/service/frontend.cc
+++ b/be/src/service/frontend.cc
@@ -25,6 +25,7 @@
#include "rpc/jni-thrift-util.h"
#include "util/backend-gflag-util.h"
#include "util/jni-util.h"
+#include "util/test-info.h"
#include "util/time.h"
#include "common/names.h"
@@ -81,7 +82,7 @@ DEFINE_string(kudu_master_hosts, "", "Specifies the default Kudu master(s). The
Frontend::Frontend() {
JniMethodDescriptor methods[] = {
- {"<init>", "([B)V", &fe_ctor_},
+ {"<init>", "([BZ)V", &fe_ctor_},
{"createExecRequest", "([B)[B", &create_exec_request_id_},
{"getExplainPlan", "([B)Ljava/lang/String;", &get_explain_plan_id_},
{"getHadoopConfig", "([B)[B", &get_hadoop_config_id_},
@@ -130,7 +131,10 @@ Frontend::Frontend() {
jbyteArray cfg_bytes;
ABORT_IF_ERROR(GetThriftBackendGflags(jni_env, &cfg_bytes));
- jobject fe = jni_env->NewObject(fe_class, fe_ctor_, cfg_bytes);
+ // Pass in whether this is a backend test, so that the Frontend can avoid certain
+ // unnecessary initialization that introduces dependencies on a running minicluster.
+ jboolean is_be_test = TestInfo::is_be_test();
+ jobject fe = jni_env->NewObject(fe_class, fe_ctor_, cfg_bytes, is_be_test);
ABORT_IF_EXC(jni_env);
ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, fe, &fe_));
}
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 1715233..d4ed406 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -281,8 +281,9 @@ public class Frontend {
private static ExecutorService checkAuthorizationPool_;
- public Frontend(AuthorizationFactory authzFactory) throws ImpalaException {
- this(authzFactory, FeCatalogManager.createFromBackendConfig());
+ public Frontend(AuthorizationFactory authzFactory, boolean isBackendTest)
+ throws ImpalaException {
+ this(authzFactory, FeCatalogManager.createFromBackendConfig(), isBackendTest);
}
/**
@@ -292,11 +293,12 @@ public class Frontend {
@VisibleForTesting
public Frontend(AuthorizationFactory authzFactory, FeCatalog testCatalog)
throws ImpalaException {
- this(authzFactory, FeCatalogManager.createForTests(testCatalog));
+ // This signature is only used for frontend tests, so pass false for isBackendTest
+ this(authzFactory, FeCatalogManager.createForTests(testCatalog), false);
}
- private Frontend(AuthorizationFactory authzFactory, FeCatalogManager catalogManager)
- throws ImpalaException {
+ private Frontend(AuthorizationFactory authzFactory, FeCatalogManager catalogManager,
+ boolean isBackendTest) throws ImpalaException {
catalogManager_ = catalogManager;
authzFactory_ = authzFactory;
@@ -323,10 +325,15 @@ public class Frontend {
impaladTableUsageTracker_ = ImpaladTableUsageTracker.createFromConfig(
BackendConfig.INSTANCE);
queryHookManager_ = QueryEventHookManager.createFromConfig(BackendConfig.INSTANCE);
- metaStoreClientPool_ = new MetaStoreClientPool(1, 0);
- if (MetastoreShim.getMajorVersion() > 2) {
- transactionKeepalive_ = new TransactionKeepalive(metaStoreClientPool_);
+ if (!isBackendTest) {
+ metaStoreClientPool_ = new MetaStoreClientPool(1, 0);
+ if (MetastoreShim.getMajorVersion() > 2) {
+ transactionKeepalive_ = new TransactionKeepalive(metaStoreClientPool_);
+ } else {
+ transactionKeepalive_ = null;
+ }
} else {
+ metaStoreClientPool_ = null;
transactionKeepalive_ = null;
}
}
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 b572f8a..3642429 100644
--- a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
+++ b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
@@ -127,7 +127,8 @@ public class JniFrontend {
/**
* Create a new instance of the Jni Frontend.
*/
- public JniFrontend(byte[] thriftBackendConfig) throws ImpalaException, TException {
+ public JniFrontend(byte[] thriftBackendConfig, boolean isBackendTest)
+ throws ImpalaException, TException {
TBackendGflags cfg = new TBackendGflags();
JniUtil.deserializeThrift(protocolFactory_, cfg, thriftBackendConfig);
@@ -140,7 +141,7 @@ public class JniFrontend {
if (cfg.is_coordinator) {
final AuthorizationFactory authzFactory =
AuthorizationUtil.authzFactoryFrom(BackendConfig.INSTANCE);
- frontend_ = new Frontend(authzFactory);
+ frontend_ = new Frontend(authzFactory, isBackendTest);
} else {
// Avoid instantiating Frontend in executor only impalads.
frontend_ = null;
[impala] 02/02: IMPALA-9701: fix data race in BTS
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f4258b5f971f90390b93aa7a2e76dd0b8a1d8825
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Mon Apr 27 16:52:40 2020 -0700
IMPALA-9701: fix data race in BTS
A benign data race in BufferedTupleStream was flagged
by TSAN.
Testing:
Reran the unit test under TSAN, it succeeded.
Change-Id: Ie2c4464adbc51bb8b0214ba0adbfa71217b87c86
Reviewed-on: http://gerrit.cloudera.org:8080/15826
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/runtime/buffered-tuple-stream.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/be/src/runtime/buffered-tuple-stream.cc b/be/src/runtime/buffered-tuple-stream.cc
index a35a89e..1ca8a92 100644
--- a/be/src/runtime/buffered-tuple-stream.cc
+++ b/be/src/runtime/buffered-tuple-stream.cc
@@ -1079,7 +1079,9 @@ void BufferedTupleStream::ReadIterator::Init(bool attach_on_read) {
valid_ = true;
rows_returned_ = 0;
DCHECK(!attach_on_read_) << "attach_on_read can only be set once";
- attach_on_read_ = attach_on_read;
+ // Only set 'attach_on_read' if needed. Otherwise, if this is the builtin
+ // iterator, a benign data race may be flagged by TSAN (see IMPALA-9701).
+ if (attach_on_read) attach_on_read_ = attach_on_read;
}
void BufferedTupleStream::ReadIterator::SetReadPage(list<Page>::iterator read_page) {