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 2017/04/20 15:35:32 UTC
[1/5] incubator-impala git commit: IMPALA-5230: fix non-functional
impalad under ASAN
Repository: incubator-impala
Updated Branches:
refs/heads/master 297e23ba9 -> a2805ca6f
IMPALA-5230: fix non-functional impalad under ASAN
The bug is that TCMalloc metrics are not initialised under
ASAN but other metrics that reference them are created. As
soon as a debug webpage tries to access that metric, Impala
crashes.
The bug was introduced by commit 955b257cfb5: "IMPALA-5073:
Part 1: add option to use mmap() for buffer pool".
Testing:
Started the cluster under ASAN, ran some query tests, checked the /memz
and /metrics pages.
Change-Id: Ic71fa57e0444f470398e16513e4ba6e7f71565e4
Reviewed-on: http://gerrit.cloudera.org:8080/6694
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/830889a6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/830889a6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/830889a6
Branch: refs/heads/master
Commit: 830889a68709db345002bbf867ea86ac10235e9d
Parents: 297e23b
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Apr 19 15:11:22 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Apr 20 02:01:10 2017 +0000
----------------------------------------------------------------------
be/src/util/memory-metrics.cc | 2 +-
be/src/util/memory-metrics.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/830889a6/be/src/util/memory-metrics.cc
----------------------------------------------------------------------
diff --git a/be/src/util/memory-metrics.cc b/be/src/util/memory-metrics.cc
index 3f2d198..5f40a69 100644
--- a/be/src/util/memory-metrics.cc
+++ b/be/src/util/memory-metrics.cc
@@ -73,7 +73,6 @@ Status impala::RegisterMemoryMetrics(MetricGroup* metrics, bool register_jvm_met
TcmallocMetric::PHYSICAL_BYTES_RESERVED =
metrics->RegisterMetric(new TcmallocMetric::PhysicalBytesMetric(
MetricDefs::Get("tcmalloc.physical-bytes-reserved")));
-#endif
// Add compound metrics that track totals across TCMalloc and the buffer pool.
// total-used should track the total physical memory in use.
@@ -86,6 +85,7 @@ Status impala::RegisterMemoryMetrics(MetricGroup* metrics, bool register_jvm_met
AggregateMemoryMetric::TOTAL_USED = metrics->RegisterMetric(
new SumGauge<uint64_t>(MetricDefs::Get("memory.total-used"), used_metrics));
+#endif
if (register_jvm_metrics) {
RETURN_IF_ERROR(JvmMetric::InitMetrics(metrics->GetOrCreateChildGroup("jvm")));
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/830889a6/be/src/util/memory-metrics.h
----------------------------------------------------------------------
diff --git a/be/src/util/memory-metrics.h b/be/src/util/memory-metrics.h
index e058bcb..83de52f 100644
--- a/be/src/util/memory-metrics.h
+++ b/be/src/util/memory-metrics.h
@@ -39,7 +39,7 @@ class AggregateMemoryMetric {
/// The sum of Tcmalloc TOTAL_BYTES_RESERVED and BufferPool SYSTEM_ALLOCATED.
/// Approximates the total amount of physical memory consumed by the backend (i.e. not
/// including JVM memory), which is either in use by queries or cached by the BufferPool
- /// or TcMalloc.
+ /// or TcMalloc. NULL when running under ASAN.
static SumGauge<uint64_t>* TOTAL_USED;
};
[5/5] incubator-impala git commit: IMPALA-5031: Remove undefined
behavior: left shift of large signed
Posted by ta...@apache.org.
IMPALA-5031: Remove undefined behavior: left shift of large signed
Shifting large positive signed values is undefined when the result is
not "representable in the corresponding unsigned type of the result
type". Left shift of unsigned values is always defined. See the C++14
standard, section 5.8 [expr.shift], paragraph 2.
Change-Id: I748697cf503e9e717a6e95250c2cbbf031c6352d
Reviewed-on: http://gerrit.cloudera.org:8080/6528
Tested-by: Impala Public Jenkins
Reviewed-by: Jim Apple <jb...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/a2805ca6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/a2805ca6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/a2805ca6
Branch: refs/heads/master
Commit: a2805ca6fa31f6e49f4bb5167225a895c35f8974
Parents: 392c4ba
Author: Jim Apple <jb...@apache.org>
Authored: Tue Mar 28 15:56:26 2017 -0700
Committer: Jim Apple <jb...@apache.org>
Committed: Thu Apr 20 15:02:36 2017 +0000
----------------------------------------------------------------------
be/src/exprs/bit-byte-functions-ir.cc | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a2805ca6/be/src/exprs/bit-byte-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/bit-byte-functions-ir.cc b/be/src/exprs/bit-byte-functions-ir.cc
index 3464174..3433cb1 100644
--- a/be/src/exprs/bit-byte-functions-ir.cc
+++ b/be/src/exprs/bit-byte-functions-ir.cc
@@ -148,7 +148,8 @@ static T RotateLeftImpl(T v, int32_t shift) {
// Handle wrapping around multiple times
shift = shift % (sizeof(T) * 8);
- return (v << shift) | BitUtil::ShiftRightLogical(v, sizeof(T) * 8 - shift);
+ return static_cast<T>((static_cast<std::make_unsigned_t<T>>(v) << shift)
+ | BitUtil::ShiftRightLogical(v, sizeof(T) * 8 - shift));
}
template<typename T>
@@ -158,15 +159,14 @@ static T RotateRightImpl(T v, int32_t shift) {
// Handle wrapping around multiple times
shift = shift % (sizeof(T) * 8);
- using UnsignedT = std::make_unsigned_t<T>;
- return BitUtil::ShiftRightLogical(v, shift)
- | (static_cast<UnsignedT>(v) << (sizeof(T) * 8 - shift));
+ return static_cast<T>(BitUtil::ShiftRightLogical(v, shift)
+ | (static_cast<std::make_unsigned_t<T>>(v) << (sizeof(T) * 8 - shift)));
}
template<typename T>
static T ShiftLeftImpl(T v, int32_t shift) {
if (shift < 0) return ShiftRightLogicalImpl(v, -shift);
- return v << shift;
+ return static_cast<T>(static_cast<std::make_unsigned_t<T>>(v) << shift);
}
// Logical right shift rather than arithmetic right shift
[2/5] incubator-impala git commit: Allow BlockingQueue and ThreadPool
to accept rvalue args
Posted by ta...@apache.org.
Allow BlockingQueue and ThreadPool to accept rvalue args
Previously the BlockingQueue and ThreadPool APIs only accepted const
lvalue references, so the argument was always copied into the
queue. Very often we create a thin wrapper for each work item we submit
to a thread pool, and will not want to use that object again, so moving
it into the pool rather than copying makes the most sense.
Note the introduction of an extra template parameter into Offer() and
BlockingPut*(). To enable perfect-forwarding (i.e. allow the methods to
accept rvalue or lvalues and pass them through), we need to use a)
rvalue references (V&&) and b) do so in a 'type-deducing
context' [1]. Having the enclosing class be template-parameterized does not
count as type-deducing, so we add the dummy V parameter and the compiler
will ensure that V is properly compatible with the original T type.
[1] http://eli.thegreenplace.net/2014/perfect-forwarding-and-universal-references-in-c/
Change-Id: I1791870576cb269e86495034f92555de48f92f10
Reviewed-on: http://gerrit.cloudera.org:8080/6442
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/c7fa4dce
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c7fa4dce
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c7fa4dce
Branch: refs/heads/master
Commit: c7fa4dceb667e7279dd04a42fcd2fba94ca5214f
Parents: 830889a
Author: Henry Robinson <he...@cloudera.com>
Authored: Mon Mar 20 14:25:19 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Apr 20 02:56:43 2017 +0000
----------------------------------------------------------------------
be/src/rpc/TAcceptQueueServer.cpp | 2 +-
be/src/util/blocking-queue.h | 25 ++++++++++++++++++-------
be/src/util/thread-pool.h | 5 +++--
3 files changed, 22 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c7fa4dce/be/src/rpc/TAcceptQueueServer.cpp
----------------------------------------------------------------------
diff --git a/be/src/rpc/TAcceptQueueServer.cpp b/be/src/rpc/TAcceptQueueServer.cpp
index 58d90f1..65fdc46 100644
--- a/be/src/rpc/TAcceptQueueServer.cpp
+++ b/be/src/rpc/TAcceptQueueServer.cpp
@@ -224,7 +224,7 @@ void TAcceptQueueServer::serve() {
shared_ptr<TTransport> client = serverTransport_->accept();
// New - the work done to setup the connection has been moved to SetupConnection.
- if (!connection_setup_pool.Offer(client)) {
+ if (!connection_setup_pool.Offer(std::move(client))) {
string errStr = string("TAcceptQueueServer: thread pool unexpectedly shut down.");
GlobalOutput(errStr.c_str());
stop_ = true;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c7fa4dce/be/src/util/blocking-queue.h
----------------------------------------------------------------------
diff --git a/be/src/util/blocking-queue.h b/be/src/util/blocking-queue.h
index a762d5d..5d88397 100644
--- a/be/src/util/blocking-queue.h
+++ b/be/src/util/blocking-queue.h
@@ -105,9 +105,12 @@ class BlockingQueue : public CacheLineAligned {
return true;
}
- /// Puts an element into the queue, waiting indefinitely until there is space.
- /// If the queue is shut down, returns false.
- bool BlockingPut(const T& val) {
+ /// Puts an element into the queue, waiting indefinitely until there is space. Rvalues
+ /// are moved into the queue, lvalues are copied. If the queue is shut down, returns
+ /// false. V is a type that is compatible with T; that is, objects of type V can be
+ /// inserted into the queue.
+ template <typename V>
+ bool BlockingPut(V&& val) {
MonotonicStopWatch timer;
boost::unique_lock<boost::mutex> write_lock(put_lock_);
@@ -120,7 +123,7 @@ class BlockingQueue : public CacheLineAligned {
if (UNLIKELY(shutdown_)) return false;
DCHECK_LT(put_list_.size(), max_elements_);
- put_list_.push_back(val);
+ Put(std::forward<V>(val));
write_lock.unlock();
get_cv_.NotifyOne();
return true;
@@ -128,8 +131,11 @@ class BlockingQueue : public CacheLineAligned {
/// Puts an element into the queue, waiting until 'timeout_micros' elapses, if there is
/// no space. If the queue is shut down, or if the timeout elapsed without being able to
- /// put the element, returns false.
- bool BlockingPutWithTimeout(const T& val, int64_t timeout_micros) {
+ /// put the element, returns false. Rvalues are moved into the queue, lvalues are
+ /// copied. V is a type that is compatible with T; that is, objects of type V can be
+ /// inserted into the queue.
+ template <typename V>
+ bool BlockingPutWithTimeout(V&& val, int64_t timeout_micros) {
MonotonicStopWatch timer;
boost::unique_lock<boost::mutex> write_lock(put_lock_);
boost::system_time wtime = boost::get_system_time() +
@@ -149,7 +155,7 @@ class BlockingQueue : public CacheLineAligned {
// another thread did in fact signal
if (SizeLocked(write_lock) >= max_elements_ || shutdown_) return false;
DCHECK_LT(put_list_.size(), max_elements_);
- put_list_.push_back(val);
+ Put(std::forward<V>(val));
write_lock.unlock();
get_cv_.NotifyOne();
return true;
@@ -193,6 +199,11 @@ class BlockingQueue : public CacheLineAligned {
return get_list_size_.Load() + put_list_.size();
}
+ /// Overloads for inserting an item into the list, depending on whether it should be
+ /// moved or copied.
+ void Put(const T& val) { put_list_.push_back(val); }
+ void Put(T&& val) { put_list_.emplace_back(std::move(val)); }
+
/// True if the BlockingQueue is being shut down. Guarded by 'put_lock_'.
bool shutdown_;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c7fa4dce/be/src/util/thread-pool.h
----------------------------------------------------------------------
diff --git a/be/src/util/thread-pool.h b/be/src/util/thread-pool.h
index c5da7bd..cbc0031 100644
--- a/be/src/util/thread-pool.h
+++ b/be/src/util/thread-pool.h
@@ -75,8 +75,9 @@ class ThreadPool : public CacheLineAligned {
//
/// Returns true if the work item was successfully added to the queue, false otherwise
/// (which typically means that the thread pool has already been shut down).
- bool Offer(const T& work) {
- return work_queue_.BlockingPut(work);
+ template <typename V>
+ bool Offer(V&& work) {
+ return work_queue_.BlockingPut(std::forward<V>(work));
}
/// Shuts the thread pool down, causing the work queue to cease accepting offered work
[3/5] incubator-impala git commit: IMPALA-5031: Apply UBSan options
to catalogd
Posted by ta...@apache.org.
IMPALA-5031: Apply UBSan options to catalogd
catalogd runs some C++ code, and that code can have undefined
behavior. This patch sets up the catalogd environment to give stack
traces and it excludes the libstdc++ undefined behavior from causing a
warning to be printed, as described in commit b41c2114f42f92441e96b4.
Change-Id: I520f8620d9b9f516ca5c55da5294de619e8e2d8f
Reviewed-on: http://gerrit.cloudera.org:8080/6652
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/634c2f7b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/634c2f7b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/634c2f7b
Branch: refs/heads/master
Commit: 634c2f7b9fcbb96f9c73a1cb6a6ce64780bd5eac
Parents: c7fa4dc
Author: Jim Apple <jb...@apache.org>
Authored: Sat Apr 15 15:09:00 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Apr 20 03:18:54 2017 +0000
----------------------------------------------------------------------
bin/start-catalogd.sh | 3 +++
1 file changed, 3 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/634c2f7b/bin/start-catalogd.sh
----------------------------------------------------------------------
diff --git a/bin/start-catalogd.sh b/bin/start-catalogd.sh
index 1046c82..18d2c6a 100755
--- a/bin/start-catalogd.sh
+++ b/bin/start-catalogd.sh
@@ -70,4 +70,7 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
fi
. ${IMPALA_HOME}/bin/set-classpath.sh
+export UBSAN_OPTIONS="print_stacktrace=1"
+UBSAN_OPTIONS="${UBSAN_OPTIONS} suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
+export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"
exec ${BINARY_BASE_DIR}/${BUILD_TYPE}/catalog/catalogd ${CATALOGD_ARGS}
[4/5] incubator-impala git commit: IMPALA-5224: remove defunct
codehaus repository
Posted by ta...@apache.org.
IMPALA-5224: remove defunct codehaus repository
The server is no longer even up, so there is no point in having it in
our poms.
Change-Id: I2310000e51c5e6d85a0fa30874629f4f19427c6c
Reviewed-on: http://gerrit.cloudera.org:8080/6678
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/392c4bad
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/392c4bad
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/392c4bad
Branch: refs/heads/master
Commit: 392c4badffa3a8ab9c605385f18cc8afb4182693
Parents: 634c2f7
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Tue Apr 18 16:21:29 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Apr 20 03:24:33 2017 +0000
----------------------------------------------------------------------
ext-data-source/api/pom.xml | 7 -------
ext-data-source/sample/pom.xml | 7 -------
ext-data-source/test/pom.xml | 7 -------
3 files changed, 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/392c4bad/ext-data-source/api/pom.xml
----------------------------------------------------------------------
diff --git a/ext-data-source/api/pom.xml b/ext-data-source/api/pom.xml
index 12f1e56..65e3c05 100644
--- a/ext-data-source/api/pom.xml
+++ b/ext-data-source/api/pom.xml
@@ -53,13 +53,6 @@
<enabled>false</enabled>
</releases>
</repository>
- <repository>
- <id>Codehaus repository</id>
- <url>http://repository.codehaus.org/</url>
- <snapshots>
- <enabled>false</enabled>
- </snapshots>
- </repository>
</repositories>
<dependencies>
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/392c4bad/ext-data-source/sample/pom.xml
----------------------------------------------------------------------
diff --git a/ext-data-source/sample/pom.xml b/ext-data-source/sample/pom.xml
index 383164b..617f7ed 100644
--- a/ext-data-source/sample/pom.xml
+++ b/ext-data-source/sample/pom.xml
@@ -53,13 +53,6 @@
<enabled>false</enabled>
</releases>
</repository>
- <repository>
- <id>Codehaus repository</id>
- <url>http://repository.codehaus.org/</url>
- <snapshots>
- <enabled>false</enabled>
- </snapshots>
- </repository>
</repositories>
<dependencies>
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/392c4bad/ext-data-source/test/pom.xml
----------------------------------------------------------------------
diff --git a/ext-data-source/test/pom.xml b/ext-data-source/test/pom.xml
index 3c3d5a1..ad4c46d 100644
--- a/ext-data-source/test/pom.xml
+++ b/ext-data-source/test/pom.xml
@@ -57,13 +57,6 @@
<enabled>false</enabled>
</releases>
</repository>
- <repository>
- <id>Codehaus repository</id>
- <url>http://repository.codehaus.org/</url>
- <snapshots>
- <enabled>false</enabled>
- </snapshots>
- </repository>
</repositories>
<dependencies>