You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bh...@apache.org on 2018/10/08 17:11:58 UTC
[2/6] impala git commit: IMPALA-2063 Remove newline characters in
query status.
IMPALA-2063 Remove newline characters in query status.
Remove extraneous whitespace at the end of strings being added to
profiles. Remove any duplicated newline characters within strings
as well. (The latter step is necessary to allow for a blanket
assertion on this in testing.)
Change-Id: I2bbd7d7fe2c6d0f3799d0e6b336710bccfef0ab1
Reviewed-on: http://gerrit.cloudera.org:8080/11425
Reviewed-by: Michael Ho <kw...@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/d3db3266
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d3db3266
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d3db3266
Branch: refs/heads/master
Commit: d3db32660537e55948f3132f778d257d078b9197
Parents: f8b2eb5
Author: Michal Ostrowski <mo...@cloudera.com>
Authored: Tue Sep 11 09:23:01 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Oct 5 22:30:44 2018 +0000
----------------------------------------------------------------------
be/src/util/runtime-profile.cc | 19 ++++++++++++-------
be/src/util/runtime-profile.h | 3 ++-
tests/query_test/test_cancellation.py | 15 +++++++++++++++
3 files changed, 29 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/d3db3266/be/src/util/runtime-profile.cc
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.cc b/be/src/util/runtime-profile.cc
index 51ab2fc..dd83ee6 100644
--- a/be/src/util/runtime-profile.cc
+++ b/be/src/util/runtime-profile.cc
@@ -27,6 +27,7 @@
#include <boost/thread/thread.hpp>
#include "common/object-pool.h"
+#include "gutil/strings/strip.h"
#include "rpc/thrift-util.h"
#include "util/coding-util.h"
#include "util/compress.h"
@@ -292,7 +293,7 @@ void RuntimeProfile::Update(const vector<TRuntimeProfileNode>& nodes, int* idx)
DCHECK(it != info_strings.end());
InfoStrings::iterator existing = info_strings_.find(key);
if (existing == info_strings_.end()) {
- info_strings_.insert(make_pair(key, it->second));
+ info_strings_.emplace(key, it->second);
info_strings_display_order_.push_back(key);
} else {
info_strings_[key] = it->second;
@@ -534,19 +535,23 @@ void RuntimeProfile::AppendInfoString(const string& key, const string& value) {
return AddInfoStringInternal(key, value, true);
}
-void RuntimeProfile::AddInfoStringInternal(
- const string& key, const string& value, bool append, bool redact) {
- const string& info = redact ? RedactCopy(value): value;
+void RuntimeProfile::AddInfoStringInternal(const string& key, string value,
+ bool append, bool redact) {
+
+ if (redact) Redact(&value);
+
+ StripTrailingWhitespace(&value);
+
lock_guard<SpinLock> l(info_strings_lock_);
InfoStrings::iterator it = info_strings_.find(key);
if (it == info_strings_.end()) {
- info_strings_.insert(make_pair(key, info));
+ info_strings_.emplace(key, std::move(value));
info_strings_display_order_.push_back(key);
} else {
if (append) {
- it->second += ", " + value;
+ it->second += ", " + std::move(value);
} else {
- it->second = info;
+ it->second = std::move(value);
}
}
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d3db3266/be/src/util/runtime-profile.h
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.h b/be/src/util/runtime-profile.h
index a6b06ba..e0048d2 100644
--- a/be/src/util/runtime-profile.h
+++ b/be/src/util/runtime-profile.h
@@ -479,8 +479,9 @@ class RuntimeProfile { // NOLINT: This struct is not packed, but there are not s
/// Implementation of AddInfoString() and AppendInfoString(). If 'append' is false,
/// implements AddInfoString(), otherwise implements AppendInfoString().
/// Redaction rules are applied on the info string if 'redact' is true.
+ /// Trailing whitspace is removed.
void AddInfoStringInternal(
- const std::string& key, const std::string& value, bool append, bool redact = false);
+ const std::string& key, std::string value, bool append, bool redact = false);
/// Name of the counter maintaining the total time.
static const std::string TOTAL_TIME_COUNTER_NAME;
http://git-wip-us.apache.org/repos/asf/impala/blob/d3db3266/tests/query_test/test_cancellation.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_cancellation.py b/tests/query_test/test_cancellation.py
index ecf3126..0958b69 100644
--- a/tests/query_test/test_cancellation.py
+++ b/tests/query_test/test_cancellation.py
@@ -152,12 +152,16 @@ class TestCancellation(ImpalaTestSuite):
def fetch_results():
threading.current_thread().fetch_results_error = None
+ threading.current_thread().query_profile = None
try:
new_client = self.create_impala_client()
new_client.fetch(query, handle)
except ImpalaBeeswaxException as e:
threading.current_thread().fetch_results_error = e
+ threading.current_thread().query_profile = \
+ self.impalad_test_service.get_thrift_profile(handle.get_handle().id)
+
thread = threading.Thread(target=fetch_results)
thread.start()
@@ -179,6 +183,17 @@ class TestCancellation(ImpalaTestSuite):
# Before accessing fetch_results_error we need to join the fetch thread
thread.join()
+ # IMPALA-2063 Cancellation tests may generate profile text that is otherwise hard
+ # to reproduce for testing mis-formatting.
+ profile = thread.query_profile
+ if profile:
+ for (k, v) in profile.nodes[1].info_strings.iteritems():
+ assert v == v.rstrip(), \
+ "Mis-formatted profile text: %s %s" % (k, v)
+ # "Plan" text may be strangely formatted.
+ assert k == 'Plan' or '\n\n' not in v, \
+ "Mis-formatted profile text: %s %s" % (k, v)
+
if thread.fetch_results_error is None:
# If the fetch rpc didn't result in CANCELLED (and auto-close the query) then
# the close rpc should have succeeded.