You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/09/24 20:52:26 UTC
[1/6] impala git commit: IMPALA-7498: Fix log spew from LocalCatalog
startup
Repository: impala
Updated Branches:
refs/heads/master 083352b1e -> 15e40a3c9
IMPALA-7498: Fix log spew from LocalCatalog startup
Frontend calls LocalCatalog#waitForCatalog() in a tight loop during
startup and ends up spewing tons of log messages if the MetaProvider
takes some time to initialize.
This is a problem for CatalogdMetaProvider implementation since it
waits on the Catalog server to send an initial update. This can take
some time depending on various factors and the logs become too noisy
during that period.
This patch adds a sleep() inside waitForCatalog() to avoid this. We
can do something fancy, like ImpaladCatalog implementation, by waiting
on a synchronized monitor and getting notified when the Catalog is
initialized, but I don't think it is worth the effort, especially since
the sleep time is pretty small (MAX_CATALOG_UPDATE_WAIT_TIME_MS = 2s).
Testing: Tested it locally by just starting the impalad catalog in local
mode and the logs look much better while waiting for the Catalog to send
the initial update.
Change-Id: I81e7cc6f464fd71afecd94a5a2b777aff47ee11b
Reviewed-on: http://gerrit.cloudera.org:8080/11472
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/d81d5086
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d81d5086
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d81d5086
Branch: refs/heads/master
Commit: d81d5086bf29bd81a30fe1ab5287615ee613c02f
Parents: 083352b
Author: Bharath Vissapragada <bh...@cloudera.com>
Authored: Wed Sep 19 11:50:44 2018 -0700
Committer: Bharath Vissapragada <bh...@cloudera.com>
Committed: Mon Sep 24 17:31:05 2018 +0000
----------------------------------------------------------------------
.../org/apache/impala/catalog/local/LocalCatalog.java | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/d81d5086/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
index 328f3b1..683ed18 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
@@ -208,10 +208,13 @@ public class LocalCatalog implements FeCatalog {
@Override
public void waitForCatalogUpdate(long timeoutMs) {
- // No-op for local catalog.
- // TODO(todd): Frontend.waitForCatalog() gets called at startup and ends
- // up being a tight loop that spews logs unless we do something to wait better
- // here.
+ if (isReady()) return;
+ // Sleep here to avoid log spew from the retry loop in Frontend.
+ try {
+ Thread.sleep(timeoutMs);
+ } catch (InterruptedException e) {
+ // Ignore
+ }
}
@Override
[6/6] impala git commit: IMPALA-589: Add sql function returning the
impalad coordinator hostname.
Posted by ta...@apache.org.
IMPALA-589: Add sql function returning the impalad coordinator hostname.
In every execution of an Impala query, one of the impalad daemons acts
as the coordinator node. In some cases, such as when using a proxy, a
user cannot predict which host will act as the coordinator. To aid in
diagnosis, we provide a sql function which returns the name of the host
on which the coordinator is running.
EXTERNAL DESCRIPTION:
Add a builtin function called coordinator(), which returns the name of
the host which is running the impalad that is acting as the coordinator
for the current query.
TESTING:
- Added a basic unit test for the new function.
- Added a unit test which simulates the case when coord_address is
unset.
- Added a query that uses coordinator() to exprs.test
- Hand tested in a development deployment.
- Ran regression tests and got a clean run.
Change-Id: I94d6e2664ba659b48df53c5c06f67b502c533e47
Reviewed-on: http://gerrit.cloudera.org:8080/11459
Reviewed-by: Thomas Marshall <th...@cmu.edu>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/15e40a3c
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/15e40a3c
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/15e40a3c
Branch: refs/heads/master
Commit: 15e40a3c9407e5f3d9cf44bfd270b2c4bbc842e9
Parents: cf7f221
Author: Andrew Sherman <as...@cloudera.com>
Authored: Tue Sep 4 10:05:48 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Sep 24 20:26:44 2018 +0000
----------------------------------------------------------------------
be/src/exprs/expr-test.cc | 27 ++++++++++++++++++++
be/src/exprs/utility-functions-ir.cc | 8 ++++++
be/src/exprs/utility-functions.h | 4 +++
common/function-registry/impala_functions.py | 1 +
docs/topics/impala_misc_functions.xml | 20 +++++++++++++++
.../queries/QueryTest/exprs.test | 6 +++++
6 files changed, 66 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index dd520ba..ab913e5 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -31,6 +31,7 @@
#include "codegen/llvm-codegen.h"
#include "common/init.h"
#include "common/object-pool.h"
+#include "exprs/anyval-util.h"
#include "exprs/is-null-predicate.h"
#include "exprs/like-predicate.h"
#include "exprs/literal.h"
@@ -65,6 +66,7 @@
#include "util/string-parser.h"
#include "util/string-util.h"
#include "util/test-info.h"
+#include "utility-functions.h"
#include "common/names.h"
@@ -4874,6 +4876,9 @@ TEST_F(ExprTest, UtilityFunctions) {
TestStringValue("version()", GetVersionString());
TestValue("sleep(100)", TYPE_BOOLEAN, true);
TestIsNull("sleep(NULL)", TYPE_BOOLEAN);
+ string hostname;
+ ASSERT_OK(GetHostname(&hostname));
+ TestStringValue("coordinator()", hostname);
// Test typeOf
TestStringValue("typeOf(!true)", "BOOLEAN");
@@ -4890,6 +4895,8 @@ TEST_F(ExprTest, UtilityFunctions) {
TestStringValue("typeOf(cast(10 as FLOAT))", "FLOAT");
TestStringValue("typeOf(cast(10 as DOUBLE))", "DOUBLE");
TestStringValue("typeOf(current_database())", "STRING");
+ TestStringValue("typeOf(version())", "STRING");
+ TestStringValue("typeOf(coordinator())", "STRING");
TestStringValue("typeOf(now())", "TIMESTAMP");
TestStringValue("typeOf(utc_timestamp())", "TIMESTAMP");
TestStringValue("typeOf(cast(10 as DECIMAL))", "DECIMAL(9,0)");
@@ -4946,6 +4953,26 @@ TEST_F(ExprTest, UtilityFunctions) {
TestIsNull("fnv_hash(NULL)", TYPE_BIGINT);
}
+// Test that UtilityFunctions::Coordinator() will return null if coord_address is unset
+TEST_F(ExprTest, CoordinatorFunction) {
+ // Make a RuntimeState where the query context does not have coord_address set.
+ // Note that this should never happen in a real impalad.
+ RuntimeState state(TQueryCtx(), ExecEnv::GetInstance());
+ MemTracker tracker;
+ MemPool mem_pool(&tracker);
+ FunctionContext::TypeDesc return_type;
+ return_type.type = FunctionContext::Type::TYPE_STRING;
+ std::vector<FunctionContext::TypeDesc> no_arguments;
+ FunctionContext* context =
+ CreateUdfTestContext(return_type, no_arguments, &state, &mem_pool);
+
+ StringVal coordinator = UtilityFunctions::Coordinator(context);
+ ASSERT_TRUE(coordinator.is_null) << "Coordinator() did not return expected null value";
+
+ UdfTestHarness::CloseContext(context);
+ state.ReleaseResources();
+}
+
TEST_F(ExprTest, MurmurHashFunction) {
string s("hello world");
int64_t expected = HashUtil::MurmurHash2_64(s.data(), s.size(),
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/be/src/exprs/utility-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/utility-functions-ir.cc b/be/src/exprs/utility-functions-ir.cc
index 9806ec3..a3335f7 100644
--- a/be/src/exprs/utility-functions-ir.cc
+++ b/be/src/exprs/utility-functions-ir.cc
@@ -171,6 +171,14 @@ StringVal UtilityFunctions::CurrentSession(FunctionContext* ctx) {
return AnyValUtil::FromString(ctx, PrintId(ctx->impl()->state()->session_id()));
}
+StringVal UtilityFunctions::Coordinator(FunctionContext* ctx) {
+ const TQueryCtx& query_ctx = ctx->impl()->state()->query_ctx();
+ // An empty string indicates the coordinator was not set in the query request.
+ return query_ctx.__isset.coord_address ?
+ AnyValUtil::FromString(ctx, query_ctx.coord_address.hostname) :
+ StringVal::null();
+}
+
template<typename T>
StringVal UtilityFunctions::TypeOf(FunctionContext* ctx, const T& /*input_val*/) {
FunctionContext::TypeDesc type_desc = *(ctx->GetArgType(0));
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/be/src/exprs/utility-functions.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/utility-functions.h b/be/src/exprs/utility-functions.h
index dbb1fb1..f78ffc7 100644
--- a/be/src/exprs/utility-functions.h
+++ b/be/src/exprs/utility-functions.h
@@ -91,6 +91,10 @@ class UtilityFunctions {
static void UuidClose(FunctionContext* ctx,
FunctionContext::FunctionStateScope scope);
+ /// Implementation of the coordinator() function.
+ /// Returns the name of the host where the coordinator is running.
+ static StringVal Coordinator(FunctionContext* ctx);
+
/// Implementation of the typeOf() function. Returns the type of the input
/// expression. input_val is not used and it is kept here in order to let
/// the compiler generate the corresponding fully-qualified function name.
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/common/function-registry/impala_functions.py
----------------------------------------------------------------------
diff --git a/common/function-registry/impala_functions.py b/common/function-registry/impala_functions.py
index b29d1a8..baf75d3 100644
--- a/common/function-registry/impala_functions.py
+++ b/common/function-registry/impala_functions.py
@@ -582,6 +582,7 @@ visible_functions = [
[['sleep'], 'BOOLEAN', ['INT'], 'impala::UtilityFunctions::Sleep'],
[['pid'], 'INT', [], 'impala::UtilityFunctions::Pid'],
[['version'], 'STRING', [], 'impala::UtilityFunctions::Version'],
+ [['coordinator'], 'STRING', [], 'impala::UtilityFunctions::Coordinator'],
[['typeOf'], 'STRING', ['BOOLEAN'], '_ZN6impala16UtilityFunctions6TypeOfIN10impala_udf10BooleanValEEENS2_9StringValEPNS2_15FunctionContextERKT_'],
[['typeOf'], 'STRING', ['TINYINT'], '_ZN6impala16UtilityFunctions6TypeOfIN10impala_udf10TinyIntValEEENS2_9StringValEPNS2_15FunctionContextERKT_'],
[['typeOf'], 'STRING', ['SMALLINT'], '_ZN6impala16UtilityFunctions6TypeOfIN10impala_udf11SmallIntValEEENS2_9StringValEPNS2_15FunctionContextERKT_'],
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/docs/topics/impala_misc_functions.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_misc_functions.xml b/docs/topics/impala_misc_functions.xml
index 7bd57d3..1b5bfd4 100644
--- a/docs/topics/impala_misc_functions.xml
+++ b/docs/topics/impala_misc_functions.xml
@@ -209,6 +209,26 @@ select uuid() from four_row_table;
</dd>
</dlentry>
+
+
+
+ <dlentry id="coordinator">
+ <dt>
+ <codeph>coordinator()</codeph>
+ </dt>
+
+ <dd>
+ <indexterm audience="hidden">coordinator() function</indexterm>
+ <b>Purpose:</b> Returns the name of the host which is running the
+ <codeph>impalad</codeph> daemon that is acting as the <codeph>coordinator</codeph>
+ for the curent query.
+ <p>
+ <b>Return type:</b> <codeph>string</codeph>
+ </p>
+ </dd>
+
+ </dlentry>
+
</dl>
</conbody>
</concept>
http://git-wip-us.apache.org/repos/asf/impala/blob/15e40a3c/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index 3b42ce4..eaf8e89 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -3007,3 +3007,9 @@ in ('0', '1', '2', '3', '4', '5', '6', '7', '8', '9')
---- TYPES
bigint
====
+---- QUERY
+# test coordinator() function in a real query
+select coordinator(), count(*) from functional.alltypesagg where int_col = 1
+---- TYPES
+string, bigint
+====
[4/6] impala git commit: Mark certain vendored JS/CSS files as
"binary" to avoid them showing up in "git grep".
Posted by ta...@apache.org.
Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
This instructs git to pretend that certain "minimized" and debugging
Bootstrap-related files are treated as binary by git tools, especially
"git grep". As a result, doing something like "git grep pause" won't
yield a wall of indecipherable JavaScript.
Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1
Reviewed-on: http://gerrit.cloudera.org:8080/11478
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/89724d8a
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/89724d8a
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/89724d8a
Branch: refs/heads/master
Commit: 89724d8a3a5f943cbc94e4c88ce23810dd20de67
Parents: 16f9437
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Wed Sep 19 12:48:52 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Sep 24 20:09:15 2018 +0000
----------------------------------------------------------------------
.gitattributes | 20 ++++++++++++++++++++
bin/rat_exclude_files.txt | 1 +
2 files changed, 21 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/89724d8a/.gitattributes
----------------------------------------------------------------------
diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..2c1789c
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,20 @@
+# Marking minified JS files prevents long lines from showing up in "git grep".
+tests/comparison/leopard/static/css/bootstrap.css binary
+tests/comparison/leopard/static/css/bootstrap.css.map binary
+tests/comparison/leopard/static/css/bootstrap.min.css binary
+tests/comparison/leopard/static/js/bootstrap.js binary
+tests/comparison/leopard/static/js/bootstrap.min.js binary
+www/bootstrap/css/bootstrap.css binary
+www/bootstrap/css/bootstrap.css.map binary
+www/bootstrap/css/bootstrap.min.css binary
+www/bootstrap/js/bootstrap.js binary
+www/bootstrap/js/bootstrap.min.js
+www/bootstrap/js/bootstrap.min.js binary
+www/d3.v3.min.js binary
+www/dagre-d3.min.js binary
+www/DataTables-1.10.12/js/dataTables.bootstrap.min.js binary
+www/DataTables-1.10.12/js/dataTables.foundation.min.js binary
+www/DataTables-1.10.12/js/dataTables.jqueryui.min.js binary
+www/DataTables-1.10.12/js/jquery.dataTables.min.js binary
+www/datatables.min.js binary
+www/jquery/jquery-1.12.4.min.js binary
http://git-wip-us.apache.org/repos/asf/impala/blob/89724d8a/bin/rat_exclude_files.txt
----------------------------------------------------------------------
diff --git a/bin/rat_exclude_files.txt b/bin/rat_exclude_files.txt
index cb5e022..4cacc8f 100644
--- a/bin/rat_exclude_files.txt
+++ b/bin/rat_exclude_files.txt
@@ -5,6 +5,7 @@
# in either its literal elements or its structure is not protected by copyright law;
# therefore, such a file does not require a license header."
.clang-format
+.gitattributes
.gitignore
*/.gitignore
*/rat_exclude_files.txt
[3/6] impala git commit: IMPALA-7589: default query options for
custom cluster
Posted by ta...@apache.org.
IMPALA-7589: default query options for custom cluster
The bug that caused the erasure coding test failure was that the
default query options specified by the test overrode the allow_erasure_coded_files
option that was added by the custom cluster test infrastructure when running
erasure coded tests.
Testing:
Manually ran a custom cluster test with and without ERASURE_CODING=true
and with --capture=no and confirmed the right arguments were passed
to start-impala-cluster.py.
Change-Id: I14f60ea8746657a731e48850b0e48300a2b7c66d
Reviewed-on: http://gerrit.cloudera.org:8080/11463
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/16f9437b
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/16f9437b
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/16f9437b
Branch: refs/heads/master
Commit: 16f9437b4b86c444e80f54fc19f35f1cb8bae920
Parents: 8c330ad
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu Sep 13 08:42:43 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Sep 24 20:02:40 2018 +0000
----------------------------------------------------------------------
tests/common/custom_cluster_test_suite.py | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/16f9437b/tests/common/custom_cluster_test_suite.py
----------------------------------------------------------------------
diff --git a/tests/common/custom_cluster_test_suite.py b/tests/common/custom_cluster_test_suite.py
index cf4654a..6e14274 100644
--- a/tests/common/custom_cluster_test_suite.py
+++ b/tests/common/custom_cluster_test_suite.py
@@ -42,6 +42,9 @@ CATALOGD_ARGS = 'catalogd_args'
# Additional args passed to the start-impala-cluster script.
START_ARGS = 'start_args'
SENTRY_CONFIG = 'sentry_config'
+# Default query options passed to the impala daemon command line. Handled separately from
+# other impala daemon arguments to allow merging multiple defaults into a single list.
+DEFAULT_QUERY_OPTIONS = 'default_query_options'
# Run with fast topic updates by default to reduce time to first query running.
DEFAULT_STATESTORE_ARGS = '--statestore_update_frequency_ms=50 \
@@ -92,7 +95,7 @@ class CustomClusterTestSuite(ImpalaTestSuite):
@staticmethod
def with_args(impalad_args=None, statestored_args=None, catalogd_args=None,
- start_args=None, sentry_config=None):
+ start_args=None, sentry_config=None, default_query_options=None):
"""Records arguments to be passed to a cluster by adding them to the decorated
method's func_dict"""
def decorate(func):
@@ -109,6 +112,8 @@ class CustomClusterTestSuite(ImpalaTestSuite):
func.func_dict[START_ARGS] = start_args
if sentry_config is not None:
func.func_dict[SENTRY_CONFIG] = sentry_config
+ if default_query_options is not None:
+ func.func_dict[DEFAULT_QUERY_OPTIONS] = default_query_options
return func
return decorate
@@ -123,7 +128,8 @@ class CustomClusterTestSuite(ImpalaTestSuite):
if SENTRY_CONFIG in method.func_dict:
self._start_sentry_service(method.func_dict[SENTRY_CONFIG])
# Start a clean new cluster before each test
- self._start_impala_cluster(cluster_args)
+ self._start_impala_cluster(
+ cluster_args, default_query_options=method.func_dict.get(DEFAULT_QUERY_OPTIONS))
super(CustomClusterTestSuite, self).setup_class()
def teardown_method(self, method):
@@ -155,7 +161,8 @@ class CustomClusterTestSuite(ImpalaTestSuite):
@classmethod
def _start_impala_cluster(cls, options, log_dir=os.getenv('LOG_DIR', "/tmp/"),
cluster_size=CLUSTER_SIZE, num_coordinators=NUM_COORDINATORS,
- use_exclusive_coordinators=False, log_level=1, expected_num_executors=CLUSTER_SIZE):
+ use_exclusive_coordinators=False, log_level=1, expected_num_executors=CLUSTER_SIZE,
+ default_query_options=None):
cls.impala_log_dir = log_dir
# We ignore TEST_START_CLUSTER_ARGS here. Custom cluster tests specifically test that
# certain custom startup arguments work and we want to keep them independent of dev
@@ -177,8 +184,16 @@ class CustomClusterTestSuite(ImpalaTestSuite):
cmd.append("--impalad_args=%s --catalogd_args=%s" %
("--pull_incremental_statistcs", "--pull_incremental_statistics"))
+ default_query_option_kvs = []
+ # Put any defaults first, then any arguments after that so they can override defaults.
if os.environ.get("ERASURE_CODING") == "true":
- cmd.append("--impalad_args=--default_query_options=allow_erasure_coded_files=true")
+ default_query_option_kvs.append(("allow_erasure_coded_files", "true"))
+ if default_query_options is not None:
+ default_query_option_kvs.extend(default_query_options)
+
+ if default_query_option_kvs:
+ options.append("--impalad_args=--default_query_options={0}".format(
+ ','.join(["{0}={1}".format(k, v) for k, v in default_query_option_kvs])))
logging.info("Starting cluster with command: %s" %
" ".join(pipes.quote(arg) for arg in cmd + options))
[5/6] impala git commit: IMPALA-7519: Support elliptic curve ssl
ciphers
Posted by ta...@apache.org.
IMPALA-7519: Support elliptic curve ssl ciphers
Thrift's SSLSocketFactory class does not support setting ciphers that
use ecdh. This patch modifies our existing subclass of
SSLSocketFactory to override the ciphers() method and enable ECDH.
The code for this was taken from be/src/kudu/security/tls_context.cc
Testing:
- Added a custom cluster test that verifies that a cluster with only
ECDH ciphers enabled works.
Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
Reviewed-on: http://gerrit.cloudera.org:8080/11376
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/cf7f221d
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/cf7f221d
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/cf7f221d
Branch: refs/heads/master
Commit: cf7f221d2fe89681d0fb357ce47204d0366e3dfe
Parents: 89724d8
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
Authored: Fri Aug 31 16:16:02 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Sep 24 20:24:26 2018 +0000
----------------------------------------------------------------------
be/src/rpc/thrift-server.cc | 45 +++++++++++++++++++++++++++-
tests/custom_cluster/test_client_ssl.py | 17 +++++++++++
2 files changed, 61 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/cf7f221d/be/src/rpc/thrift-server.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-server.cc b/be/src/rpc/thrift-server.cc
index c5b0e36..62b11fd 100644
--- a/be/src/rpc/thrift-server.cc
+++ b/be/src/rpc/thrift-server.cc
@@ -20,6 +20,7 @@
#include <boost/thread/mutex.hpp>
#include <boost/uuid/uuid_io.hpp>
+#include <openssl/err.h>
#include <thrift/concurrency/Thread.h>
#include <thrift/concurrency/ThreadManager.h>
#include <thrift/protocol/TBinaryProtocol.h>
@@ -31,6 +32,7 @@
#include <sstream>
#include "gen-cpp/Types_types.h"
+#include "kudu/security/openssl_util.h"
#include "rpc/TAcceptQueueServer.h"
#include "rpc/auth-provider.h"
#include "rpc/authentication.h"
@@ -71,6 +73,10 @@ map<string, SSLProtocol> SSLProtoVersions::PROTO_MAP = {
{"tlsv1.1", TLSv1_1},
{"tlsv1", TLSv1_0}};
+// A generic wrapper for OpenSSL structures.
+template <typename T>
+using c_unique_ptr = std::unique_ptr<T, std::function<void(T*)>>;
+
Status SSLProtoVersions::StringToProtocol(const string& in, SSLProtocol* protocol) {
for (const auto& proto : SSLProtoVersions::PROTO_MAP) {
if (iequals(in, proto.first)) {
@@ -352,8 +358,45 @@ class ImpalaSslSocketFactory : public TSSLSocketFactory {
ImpalaSslSocketFactory(SSLProtocol version, const string& password)
: TSSLSocketFactory(version), password_(password) {}
+ void ciphers(const string& enable) override {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
+ TSSLSocketFactory::ciphers(enable);
+
+ // The following was taken from be/src/kudu/security/tls_context.cc, bugs fixed here
+ // may also need to be fixed there.
+ // Enable ECDH curves. For OpenSSL 1.1.0 and up, this is done automatically.
+#ifndef OPENSSL_NO_ECDH
+#if OPENSSL_VERSION_NUMBER < 0x10002000L
+ // OpenSSL 1.0.1 and below only support setting a single ECDH curve at once.
+ // We choose prime256v1 because it's the first curve listed in the "modern
+ // compatibility" section of the Mozilla Server Side TLS recommendations,
+ // accessed Feb. 2017.
+ c_unique_ptr<EC_KEY> ecdh{
+ EC_KEY_new_by_curve_name(NID_X9_62_prime256v1), &EC_KEY_free};
+ if (ecdh == nullptr) {
+ throw TSSLException(
+ "failed to create prime256v1 curve: " + kudu::security::GetOpenSSLErrors());
+ }
+
+ int rc = SSL_CTX_set_tmp_ecdh(ctx_->get(), ecdh.get());
+ if (rc <= 0) {
+ throw new TSSLException(
+ "failed to set ECDH curve: " + kudu::security::GetOpenSSLErrors());
+ }
+#elif OPENSSL_VERSION_NUMBER < 0x10100000L
+ // OpenSSL 1.0.2 provides the set_ecdh_auto API which internally figures out
+ // the best curve to use.
+ int rc = SSL_CTX_set_ecdh_auto(ctx_->get(), 1);
+ if (rc <= 0) {
+ throw TSSLException(
+ "failed to configure ECDH support: " + kudu::security::GetOpenSSLErrors());
+ }
+#endif
+#endif
+ }
+
protected:
- virtual void getPassword(string& output, int size) {
+ virtual void getPassword(string& output, int size) override {
output = password_;
if (output.size() > size) output.resize(size);
}
http://git-wip-us.apache.org/repos/asf/impala/blob/cf7f221d/tests/custom_cluster/test_client_ssl.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_client_ssl.py b/tests/custom_cluster/test_client_ssl.py
index 811d32e..cbc2f74 100644
--- a/tests/custom_cluster/test_client_ssl.py
+++ b/tests/custom_cluster/test_client_ssl.py
@@ -107,6 +107,23 @@ class TestClientSsl(CustomClusterTestSuite):
assert "Query Status: Cancelled" in result.stdout
assert impalad.wait_for_num_in_flight_queries(0)
+ # Test that the shell can connect to a ECDH only cluster.
+ TLS_ECDH_ARGS = ("--ssl_client_ca_certificate=%s/server-cert.pem "
+ "--ssl_server_certificate=%s/server-cert.pem "
+ "--ssl_private_key=%s/server-key.pem "
+ "--hostname=localhost " # Required to match hostname in certificate"
+ "--ssl_cipher_list=ECDHE-RSA-AES128-GCM-SHA256 "
+ % (CERT_DIR, CERT_DIR, CERT_DIR))
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(impalad_args=TLS_ECDH_ARGS,
+ statestored_args=TLS_ECDH_ARGS,
+ catalogd_args=TLS_ECDH_ARGS)
+ @pytest.mark.skipif(HAS_LEGACY_OPENSSL, reason=SKIP_SSL_MSG)
+ def test_tls_ecdh(self, vector):
+ self._verify_negative_cases()
+ self._validate_positive_cases("%s/server-cert.pem" % self.CERT_DIR)
+
# Test that the shell can connect to a TLS1.2 only cluster, and for good measure
# restrict the cipher suite to just one choice.
TLS_V12_ARGS = ("--ssl_client_ca_certificate=%s/server-cert.pem "
[2/6] impala git commit: IMPALA-7527: add fetch-from-catalogd cache
info to profile
Posted by ta...@apache.org.
IMPALA-7527: add fetch-from-catalogd cache info to profile
This patch adds a Java wrapper for a RuntimeProfile object. The wrapper
supports some basic operations like non-hierarchical counters and
informational strings.
During planning, a profile is created, and passed back to the backend as
part of the ExecRequest. The backend then updates the query profile
based on the info emitted from the frontend.
This patch also adds the first use case for this profile information:
the CatalogdMetaProvider emits counters for cache hits, misses, and
fetch times, broken down by metadata category.
The emitted profile is a bit of a superset of the existing 'timeline'
functionality. However, it seems that some tools may parse the timeline
in its current location in the profile, so moving it might be
incompatible. I elected to leave that alone for now and just emit
counters in the new profile.
Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Reviewed-on: http://gerrit.cloudera.org:8080/11388
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/8c330adf
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/8c330adf
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/8c330adf
Branch: refs/heads/master
Commit: 8c330adf409aa74857b23ba345f0c710a1f25a32
Parents: d81d508
Author: Todd Lipcon <to...@apache.org>
Authored: Tue Sep 4 19:07:12 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Sep 24 19:52:44 2018 +0000
----------------------------------------------------------------------
be/src/service/client-request-state.cc | 10 ++
be/src/service/client-request-state.h | 8 +
be/src/service/impala-server.cc | 1 +
be/src/util/runtime-profile.h | 2 +-
common/thrift/Frontend.thrift | 4 +
.../catalog/local/CatalogdMetaProvider.java | 92 ++++++++++-
.../org/apache/impala/service/Frontend.java | 13 +-
.../apache/impala/service/FrontendProfile.java | 163 +++++++++++++++++++
.../catalog/local/CatalogdMetaProviderTest.java | 20 +++
tests/custom_cluster/test_local_catalog.py | 17 ++
10 files changed, 315 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/be/src/service/client-request-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index 0db511d..4880ac6 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -75,6 +75,7 @@ ClientRequestState::ClientRequestState(
coord_exec_called_(false),
// Profile is assigned name w/ id after planning
profile_(RuntimeProfile::Create(&profile_pool_, "Query")),
+ frontend_profile_(RuntimeProfile::Create(&profile_pool_, "Frontend")),
server_profile_(RuntimeProfile::Create(&profile_pool_, "ImpalaServer")),
summary_profile_(RuntimeProfile::Create(&profile_pool_, "Summary")),
frontend_(frontend),
@@ -116,6 +117,8 @@ ClientRequestState::ClientRequestState(
"Sql Statement", query_ctx_.client_request.stmt);
summary_profile_->AddInfoString("Coordinator",
TNetworkAddressToString(exec_env->GetThriftBackendAddress()));
+
+ profile_->AddChild(frontend_profile_);
}
ClientRequestState::~ClientRequestState() {
@@ -136,6 +139,13 @@ Status ClientRequestState::SetResultCache(QueryResultSet* cache,
return Status::OK();
}
+void ClientRequestState::SetFrontendProfile(TRuntimeProfileNode profile) {
+ // Should we defer creating and adding the child until here? probably.
+ TRuntimeProfileTree prof_tree;
+ prof_tree.nodes.emplace_back(std::move(profile));
+ frontend_profile_->Update(prof_tree);
+}
+
Status ClientRequestState::Exec(TExecRequest* exec_request) {
MarkActive();
exec_request_ = *exec_request;
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/be/src/service/client-request-state.h
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.h b/be/src/service/client-request-state.h
index b524409..0f6f514 100644
--- a/be/src/service/client-request-state.h
+++ b/be/src/service/client-request-state.h
@@ -66,6 +66,11 @@ class ClientRequestState {
~ClientRequestState();
+ /// Sets the profile that is produced by the frontend. The frontend creates the
+ /// profile during planning and returns it to the backend via TExecRequest,
+ /// which then sets the frontend profile.
+ void SetFrontendProfile(TRuntimeProfileNode profile);
+
/// Based on query type, this either initiates execution of a exec_request or submits
/// the query to the Admission controller for asynchronous admission control. When this
/// returns the operation state is either RUNNING_STATE or PENDING_STATE.
@@ -352,6 +357,8 @@ class ClientRequestState {
/// The ClientRequestState builds three separate profiles.
/// * profile_ is the top-level profile which houses the other
/// profiles, plus the query timeline
+ /// * frontend_profile_ is the profile emitted by the frontend
+ // during planning.
/// * summary_profile_ contains mostly static information about the
/// query, including the query statement, the plan and the user who submitted it.
/// * server_profile_ tracks time spent inside the ImpalaServer,
@@ -370,6 +377,7 @@ class ClientRequestState {
/// - Query Status
/// - Error logs
RuntimeProfile* const profile_;
+ RuntimeProfile* const frontend_profile_;
RuntimeProfile* const server_profile_;
RuntimeProfile* const summary_profile_;
RuntimeProfile::Counter* row_materialization_timer_;
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 0999b5e..091e00f 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -951,6 +951,7 @@ Status ImpalaServer::ExecuteInternal(
(*request_state)->set_user_profile_access(result.user_has_profile_access);
(*request_state)->summary_profile()->AddEventSequence(
result.timeline.name, result.timeline);
+ (*request_state)->SetFrontendProfile(result.profile);
if (result.__isset.result_set_metadata) {
(*request_state)->set_result_metadata(result.result_set_metadata);
}
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/be/src/util/runtime-profile.h
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.h b/be/src/util/runtime-profile.h
index cae2462..a6b06ba 100644
--- a/be/src/util/runtime-profile.h
+++ b/be/src/util/runtime-profile.h
@@ -263,7 +263,7 @@ class RuntimeProfile { // NOLINT: This struct is not packed, but there are not s
Counter* inactive_timer() { return counter_map_[INACTIVE_TIME_COUNTER_NAME]; }
int64_t local_time() { return local_time_ns_; }
- /// Prints the counters in a name: value format.
+ /// Prints the contents of the profile in a name: value format.
/// Does not hold locks when it makes any function calls.
void PrettyPrint(std::ostream* s, const std::string& prefix="") const;
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/common/thrift/Frontend.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/Frontend.thrift b/common/thrift/Frontend.thrift
index a7d56d7..9419b17 100644
--- a/common/thrift/Frontend.thrift
+++ b/common/thrift/Frontend.thrift
@@ -595,12 +595,16 @@ struct TExecRequest {
10: optional TSetQueryOptionRequest set_query_option_request
// Timeline of planner's operation, for profiling
+ // TODO(todd): should integrate this with the 'profile' member instead.
11: optional RuntimeProfile.TEventSequence timeline
// If false, the user that runs this statement doesn't have access to the runtime
// profile. For example, a user can't access the runtime profile of a query
// that has a view for which the user doesn't have access to the underlying tables.
12: optional bool user_has_profile_access
+
+ // Profile information from the planning process.
+ 13: optional RuntimeProfile.TRuntimeProfileNode profile
}
// Parameters to FeSupport.cacheJar().
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
index ca9a765..bfa9133 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
@@ -48,6 +48,7 @@ import org.apache.impala.common.InternalException;
import org.apache.impala.common.Pair;
import org.apache.impala.common.Reference;
import org.apache.impala.service.FeSupport;
+import org.apache.impala.service.FrontendProfile;
import org.apache.impala.thrift.CatalogLookupStatus;
import org.apache.impala.thrift.TBackendGflags;
import org.apache.impala.thrift.TCatalogInfoSelector;
@@ -66,6 +67,7 @@ import org.apache.impala.thrift.TPartialPartitionInfo;
import org.apache.impala.thrift.TTable;
import org.apache.impala.thrift.TTableInfoSelector;
import org.apache.impala.thrift.TUniqueId;
+import org.apache.impala.thrift.TUnit;
import org.apache.impala.thrift.TUpdateCatalogCacheRequest;
import org.apache.impala.thrift.TUpdateCatalogCacheResponse;
import org.apache.impala.util.ListMap;
@@ -81,6 +83,7 @@ import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
@@ -185,6 +188,25 @@ public class CatalogdMetaProvider implements MetaProvider {
*/
private static final Object DB_LIST_CACHE_KEY = new Object();
+ private static final String CATALOG_FETCH_PREFIX = "CatalogFetch";
+ private static final String DB_LIST_STATS_CATEGORY = "DatabaseList";
+ private static final String DB_METADATA_STATS_CATEGORY = "Databases";
+ private static final String TABLE_NAMES_STATS_CATEGORY = "TableNames";
+ private static final String TABLE_METADATA_CACHE_CATEGORY = "Tables";
+ private static final String PARTITION_LIST_STATS_CATEGORY = "PartitionLists";
+ private static final String PARTITIONS_STATS_CATEGORY = "Partitions";
+ private static final String COLUMN_STATS_STATS_CATEGORY = "ColumnStats";
+ private static final String GLOBAL_CONFIGURATION_STATS_CATEGORY = "Config";
+ private static final String FUNCTION_LIST_STATS_CATEGORY = "FunctionLists";
+ private static final String FUNCTIONS_STATS_CATEGORY = "Functions";
+ private static final String RPC_STATS_CATEGORY = "RPCs";
+ private static final String RPC_REQUESTS =
+ CATALOG_FETCH_PREFIX + "." + RPC_STATS_CATEGORY + ".Requests";
+ private static final String RPC_BYTES =
+ CATALOG_FETCH_PREFIX + "." + RPC_STATS_CATEGORY + ".Bytes";
+ private static final String RPC_TIME =
+ CATALOG_FETCH_PREFIX + "." + RPC_STATS_CATEGORY + ".Time";
+
/**
* File descriptors store replicas using a compressed format that references hosts
* by index in a "host index" list rather than by their full addresses. Since we cache
@@ -290,11 +312,20 @@ public class CatalogdMetaProvider implements MetaProvider {
TGetPartialCatalogObjectRequest req)
throws TException {
TGetPartialCatalogObjectResponse resp;
- byte[] ret;
+ byte[] ret = null;
+ Stopwatch sw = new Stopwatch().start();
try {
ret = FeSupport.GetPartialCatalogObject(new TSerializer().serialize(req));
} catch (InternalException e) {
throw new TException(e);
+ } finally {
+ sw.stop();
+ FrontendProfile profile = FrontendProfile.getCurrentOrNull();
+ if (profile != null) {
+ profile.addToCounter(RPC_REQUESTS, TUnit.NONE, 1);
+ profile.addToCounter(RPC_BYTES, TUnit.BYTES, ret == null ? 0 : ret.length);
+ profile.addToCounter(RPC_TIME, TUnit.TIME_MS, sw.elapsed(TimeUnit.MILLISECONDS));
+ }
}
resp = new TGetPartialCatalogObjectResponse();
new TDeserializer().deserialize(resp, ret);
@@ -343,13 +374,15 @@ public class CatalogdMetaProvider implements MetaProvider {
@SuppressWarnings("unchecked")
private <CacheKeyType, ValueType> ValueType loadWithCaching(String itemString,
- CacheKeyType key, final Callable<ValueType> loadCallable) throws TException {
+ String statsCategory, CacheKeyType key,
+ final Callable<ValueType> loadCallable) throws TException {
// TODO(todd): there a race here if an invalidation comes in while we are
// fetching here. Perhaps we need some locking, or need to remember the
// version numbers of the invalidation messages and ensure that we don't
// 'put' an element with a too-old version? See:
// https://softwaremill.com/race-condition-cache-guava-caffeine/
final Reference<Boolean> hit = new Reference<>(true);
+ Stopwatch sw = new Stopwatch().start();
try {
return (ValueType)cache_.get(key, new Callable<ValueType>() {
@Override
@@ -362,13 +395,39 @@ public class CatalogdMetaProvider implements MetaProvider {
Throwables.propagateIfPossible(e.getCause(), TException.class);
throw new RuntimeException(e);
} finally {
+ sw.stop();
+ addStatsToProfile(statsCategory, /*numHits=*/hit.getRef() ? 1 : 0,
+ /*numMisses=*/hit.getRef() ? 0 : 1, sw);
LOG.trace("Request for {}: {}", itemString, hit.getRef() ? "hit" : "miss");
}
}
+ /**
+ * Adds basic statistics to the query's profile when accessing cache entries.
+ * For each cache request, the number of hits, misses, and elapsed time is aggregated.
+ * Cache requests for different types of cache entries, such as function names vs.
+ * table names, are differentiated by a 'statsCategory'.
+ */
+ private void addStatsToProfile(String statsCategory, int numHits, int numMisses,
+ Stopwatch stopwatch) {
+ FrontendProfile profile = FrontendProfile.getCurrentOrNull();
+ if (profile == null) return;
+ final String prefix = CATALOG_FETCH_PREFIX + "." +
+ Preconditions.checkNotNull(statsCategory) + ".";
+ profile.addToCounter(prefix + "Requests", TUnit.NONE, numHits + numMisses);;
+ profile.addToCounter(prefix + "Time", TUnit.TIME_MS,
+ stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ if (numHits > 0) {
+ profile.addToCounter(prefix + "Hits", TUnit.NONE, numHits);
+ }
+ if (numMisses > 0) {
+ profile.addToCounter(prefix + "Misses", TUnit.NONE, numMisses);
+ }
+ }
+
@Override
public ImmutableList<String> loadDbList() throws TException {
- return loadWithCaching("database list", DB_LIST_CACHE_KEY,
+ return loadWithCaching("database list", DB_LIST_STATS_CATEGORY, DB_LIST_CACHE_KEY,
new Callable<ImmutableList<String>>() {
@Override
public ImmutableList<String> call() throws Exception {
@@ -415,6 +474,7 @@ public class CatalogdMetaProvider implements MetaProvider {
@Override
public Database loadDb(final String dbName) throws TException {
return loadWithCaching("database metadata for " + dbName,
+ DB_METADATA_STATS_CATEGORY,
new DbCacheKey(dbName, DbCacheKey.DbInfoType.HMS_METADATA),
new Callable<Database>() {
@Override
@@ -433,6 +493,7 @@ public class CatalogdMetaProvider implements MetaProvider {
public ImmutableList<String> loadTableNames(final String dbName)
throws MetaException, UnknownDBException, TException {
return loadWithCaching("table names for database " + dbName,
+ TABLE_NAMES_STATS_CATEGORY,
new DbCacheKey(dbName, DbCacheKey.DbInfoType.TABLE_NAMES),
new Callable<ImmutableList<String>>() {
@Override
@@ -474,6 +535,7 @@ public class CatalogdMetaProvider implements MetaProvider {
TableCacheKey cacheKey = new TableCacheKey(dbName, tableName);
TableMetaRefImpl ref = loadWithCaching(
"table metadata for " + dbName + "." + tableName,
+ TABLE_METADATA_CACHE_CATEGORY,
cacheKey,
new Callable<TableMetaRefImpl>() {
@Override
@@ -493,6 +555,7 @@ public class CatalogdMetaProvider implements MetaProvider {
@Override
public List<ColumnStatisticsObj> loadTableColumnStatistics(final TableMetaRef table,
List<String> colNames) throws TException {
+ Stopwatch sw = new Stopwatch().start();
List<ColumnStatisticsObj> ret = Lists.newArrayListWithCapacity(colNames.size());
// Look up in cache first, keeping track of which ones are missing.
// We can't use 'loadWithCaching' since we need to fetch several entries batched
@@ -534,6 +597,9 @@ public class CatalogdMetaProvider implements MetaProvider {
NEGATIVE_COLUMN_STATS_SENTINEL);
}
}
+ sw.stop();
+ addStatsToProfile(COLUMN_STATS_STATS_CATEGORY,
+ hitCount + negativeHitCount, missingCols.size(), sw);
LOG.trace("Request for column stats of {}: hit {}/ neg hit {} / miss {}",
table, hitCount, negativeHitCount, missingCols.size());
return ret;
@@ -544,8 +610,8 @@ public class CatalogdMetaProvider implements MetaProvider {
public List<PartitionRef> loadPartitionList(final TableMetaRef table)
throws TException {
PartitionListCacheKey key = new PartitionListCacheKey((TableMetaRefImpl) table);
- return (List<PartitionRef>) loadWithCaching(
- "partition list for " + table, key, new Callable<List<PartitionRef>>() {
+ return (List<PartitionRef>) loadWithCaching("partition list for " + table,
+ PARTITION_LIST_STATS_CATEGORY, key, new Callable<List<PartitionRef>>() {
/** Called to load cache for cache misses */
@Override
public List<PartitionRef> call() throws Exception {
@@ -574,13 +640,13 @@ public class CatalogdMetaProvider implements MetaProvider {
throws MetaException, TException {
Preconditions.checkArgument(table instanceof TableMetaRefImpl);
TableMetaRefImpl refImpl = (TableMetaRefImpl)table;
-
+ Stopwatch sw = new Stopwatch().start();
// Load what we can from the cache.
Map<PartitionRef, PartitionMetadata> refToMeta = loadPartitionsFromCache(refImpl,
hostIndex, partitionRefs);
- LOG.trace("Request for partitions of {}: hit {}/{}", table, refToMeta.size(),
- partitionRefs.size());
+ final int numHits = refToMeta.size();
+ final int numMisses = partitionRefs.size() - numHits;
// Load the remainder from the catalogd.
List<PartitionRef> missingRefs = Lists.newArrayList();
@@ -594,6 +660,10 @@ public class CatalogdMetaProvider implements MetaProvider {
// Write back to the cache.
storePartitionsInCache(refImpl, hostIndex, fromCatalogd);
}
+ sw.stop();
+ addStatsToProfile(PARTITIONS_STATS_CATEGORY, refToMeta.size(), numMisses, sw);
+ LOG.trace("Request for partitions of {}: hit {}/{}", table, refToMeta.size(),
+ partitionRefs.size());
// Convert the returned map to be by-name instead of by-ref.
Map<String, PartitionMetadata> nameToMeta = Maps.newHashMapWithExpectedSize(
@@ -724,7 +794,9 @@ public class CatalogdMetaProvider implements MetaProvider {
@Override
public String loadNullPartitionKeyValue() throws MetaException, TException {
return (String) loadWithCaching("null partition key value",
- NULL_PARTITION_KEY_VALUE_CACHE_KEY, new Callable<String>() {
+ GLOBAL_CONFIGURATION_STATS_CATEGORY,
+ NULL_PARTITION_KEY_VALUE_CACHE_KEY,
+ new Callable<String>() {
/** Called to load cache for cache misses */
@Override
public String call() throws Exception {
@@ -736,6 +808,7 @@ public class CatalogdMetaProvider implements MetaProvider {
@Override
public List<String> loadFunctionNames(final String dbName) throws TException {
return loadWithCaching("function names for database " + dbName,
+ FUNCTION_LIST_STATS_CATEGORY,
new DbCacheKey(dbName, DbCacheKey.DbInfoType.FUNCTION_NAMES),
new Callable<ImmutableList<String>>() {
@Override
@@ -755,6 +828,7 @@ public class CatalogdMetaProvider implements MetaProvider {
final String functionName) throws TException {
ImmutableList<TFunction> thriftFuncs = loadWithCaching(
"function " + dbName + "." + functionName,
+ FUNCTIONS_STATS_CATEGORY,
new FunctionsCacheKey(dbName, functionName),
new Callable<ImmutableList<TFunction>>() {
@Override
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/fe/src/main/java/org/apache/impala/service/Frontend.java
----------------------------------------------------------------------
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 22a1b70..b7a04ef 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -1048,11 +1048,14 @@ public class Frontend {
throws ImpalaException {
// Timeline of important events in the planning process, used for debugging
// and profiling.
- EventSequence timeline = new EventSequence("Query Compilation");
- TExecRequest result = getTExecRequest(queryCtx, timeline, explainString);
- timeline.markEvent("Planning finished");
- result.setTimeline(timeline.toThrift());
- return result;
+ try (FrontendProfile.Scope scope = FrontendProfile.createNewWithScope()) {
+ EventSequence timeline = new EventSequence("Query Compilation");
+ TExecRequest result = getTExecRequest(queryCtx, timeline, explainString);
+ timeline.markEvent("Planning finished");
+ result.setTimeline(timeline.toThrift());
+ result.setProfile(FrontendProfile.getCurrent().emitAsThrift());
+ return result;
+ }
}
private TExecRequest getTExecRequest(TQueryCtx queryCtx, EventSequence timeline,
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/fe/src/main/java/org/apache/impala/service/FrontendProfile.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/FrontendProfile.java b/fe/src/main/java/org/apache/impala/service/FrontendProfile.java
new file mode 100644
index 0000000..3344bf6
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/service/FrontendProfile.java
@@ -0,0 +1,163 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.impala.service;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import org.apache.impala.thrift.TCounter;
+import org.apache.impala.thrift.TRuntimeProfileNode;
+import org.apache.impala.thrift.TUnit;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.errorprone.annotations.concurrent.GuardedBy;
+
+/**
+ * Wrapper class for creating a runtime profile within the frontend.
+ *
+ * In order to avoid plumbing an object through all code that might want to emit counters
+ * into the profile, this class provides some support for storing a current profile in
+ * a thread-local variable.
+ *
+ * This class is thread-safe.
+ */
+@ThreadSafe
+public class FrontendProfile {
+ private static final String ROOT_COUNTER_NAME = "";
+
+ private static ThreadLocal<FrontendProfile> THREAD_LOCAL =
+ new ThreadLocal<>();
+
+ @GuardedBy("this")
+ private TRuntimeProfileNode profile_;
+
+ /**
+ * Name-based access to the counters in the profile_.counters List<TCounter>.
+ */
+ @GuardedBy("this")
+ private final Map<String, TCounter> countersByName_ = new HashMap<>();
+
+ FrontendProfile() {
+ profile_ = new TRuntimeProfileNode("Frontend",
+ /*num_children=*/ 0,
+ /*counters=*/new ArrayList<>(),
+ /*metadata=*/-1L, // TODO(todd) what is this used for? why is it required?
+ /*indent=*/false,
+ /*info_strings=*/new HashMap<>(),
+ /*info_strings_display_order*/new ArrayList<>(),
+ /*child_counters_map=*/ImmutableMap.of(ROOT_COUNTER_NAME, new HashSet<>()));
+ }
+
+ /**
+ * Create a new profile, setting it as the current thread-local profile for the
+ * length of the current scope. This is meant to be used in a try-with-resources
+ * statement. Supports at most one scope per thread. No nested scopes are currently
+ * allowed.
+ */
+ public static Scope createNewWithScope() {
+ return new Scope(new FrontendProfile());
+ }
+
+ /**
+ * Get the profile attached to the current thread, throw IllegalStateException if there
+ * is none.
+ */
+ @Nonnull
+ public static FrontendProfile getCurrent() {
+ FrontendProfile prof = THREAD_LOCAL.get();
+ Preconditions.checkState(prof != null, "no profile in scope");
+ return prof;
+ }
+
+ /**
+ * Get the profile attached to the current thread, or null if there is no current
+ * profile.
+ */
+ @Nullable
+ public static FrontendProfile getCurrentOrNull() {
+ return THREAD_LOCAL.get();
+ }
+
+ /**
+ * Return the profile in Thrift format. This may be called only once, and after it is
+ * called, no further methods may be used on this PlannerProfile object. Any attempts
+ * to do so will result in IllegalStateExceptions.
+ */
+ public synchronized TRuntimeProfileNode emitAsThrift() {
+ Preconditions.checkState(profile_ != null, "already emitted profile");
+ TRuntimeProfileNode ret = profile_;
+ profile_ = null;
+ return ret;
+ }
+
+ /**
+ * Add an informational key/value string pair to the profile. These are written out
+ * as is to the user. Subsequent calls with the same key will overwrite previous ones.
+ */
+ public synchronized void addInfoString(String key, String val) {
+ Preconditions.checkState(profile_ != null, "already emitted profile");
+ Preconditions.checkNotNull(key);
+ Preconditions.checkNotNull(val);
+ if (profile_.getInfo_strings().put(key, val) == null) {
+ // If it's a new info string instead of replacing an existing one,
+ // we need to also include it in the 'ordering' list.
+ profile_.getInfo_strings_display_order().add(key);
+ }
+ }
+
+ /**
+ * Add 'delta' to the counter with the given name and unit. Counters are created
+ * on-demand.
+ */
+ public synchronized void addToCounter(String name, TUnit unit, long delta) {
+ Preconditions.checkState(profile_ != null, "already emitted profile");
+ TCounter counter = countersByName_.get(Preconditions.checkNotNull(name));
+ if (counter == null) {
+ // Need to create the counter.
+ counter = new TCounter(name, unit, 0);
+ countersByName_.put(name, counter);
+ profile_.counters.add(counter);
+ // Currently we don't support hierarchical counters in the frontend.
+ profile_.child_counters_map.get(ROOT_COUNTER_NAME).add(name);
+ }
+ counter.value += delta;
+ }
+
+
+ public static class Scope implements AutoCloseable {
+ private final FrontendProfile oldThreadLocalValue_;
+
+ private Scope(FrontendProfile profile) {
+ oldThreadLocalValue_ = THREAD_LOCAL.get();
+ // TODO: remove when allowing nested scopes.
+ Preconditions.checkState(oldThreadLocalValue_ == null);
+ THREAD_LOCAL.set(profile);
+ }
+
+ @Override
+ public void close() {
+ THREAD_LOCAL.set(oldThreadLocalValue_);
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
index 74264c6..de6dd07 100644
--- a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
@@ -19,6 +19,7 @@ package org.apache.impala.catalog.local;
import static org.junit.Assert.*;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -31,11 +32,13 @@ import org.apache.impala.catalog.local.MetaProvider.PartitionRef;
import org.apache.impala.catalog.local.MetaProvider.TableMetaRef;
import org.apache.impala.common.Pair;
import org.apache.impala.service.FeSupport;
+import org.apache.impala.service.FrontendProfile;
import org.apache.impala.thrift.TBackendGflags;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.TDatabase;
import org.apache.impala.thrift.TNetworkAddress;
+import org.apache.impala.thrift.TRuntimeProfileNode;
import org.apache.impala.thrift.TTable;
import org.apache.impala.util.ListMap;
import org.junit.Test;
@@ -221,4 +224,21 @@ public class CatalogdMetaProviderTest {
assertEquals(0, stats.hitCount());
assertEquals(1, stats.missCount());
}
+
+ @Test
+ public void testProfile() throws Exception {
+ FrontendProfile profile;
+ try (FrontendProfile.Scope scope = FrontendProfile.createNewWithScope()) {
+ provider_.loadTable("functional", "alltypes");
+ profile = FrontendProfile.getCurrent();
+ }
+ TRuntimeProfileNode prof = profile.emitAsThrift();
+ assertEquals(3, prof.counters.size());
+ Collections.sort(prof.counters);
+ assertEquals("TCounter(name:CatalogFetch.Tables.Hits, unit:NONE, value:1)",
+ prof.counters.get(0).toString());
+ assertEquals("TCounter(name:CatalogFetch.Tables.Requests, unit:NONE, value:1)",
+ prof.counters.get(1).toString());
+ assertEquals("CatalogFetch.Tables.Time", prof.counters.get(2).name);
+ }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/impala/blob/8c330adf/tests/custom_cluster/test_local_catalog.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_local_catalog.py b/tests/custom_cluster/test_local_catalog.py
index 5eaf4ac..d6bd9b6 100644
--- a/tests/custom_cluster/test_local_catalog.py
+++ b/tests/custom_cluster/test_local_catalog.py
@@ -210,3 +210,20 @@ class TestCompactCatalogUpdates(CustomClusterTestSuite):
finally:
client1.close()
client2.close()
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ impalad_args="--use_local_catalog=true",
+ catalogd_args="--catalog_topic_mode=minimal")
+ def test_cache_profile_metrics(self):
+ """
+ Test that profile output includes impalad local cache metrics.
+ """
+ try:
+ client = self.cluster.impalads[0].service.create_beeswax_client()
+ query = "select count(*) from functional.alltypes"
+ ret = self.execute_query_expect_success(client, query)
+ assert ret.runtime_profile.count("Frontend:") == 1
+ assert ret.runtime_profile.count("CatalogFetch") > 1
+ finally:
+ client.close()