You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jr...@apache.org on 2018/01/13 07:50:26 UTC
[1/4] impala git commit: IMPALA-5528: Upgrade GPerfTools to 2.6.3 and
tune TCMalloc for KRPC
Repository: impala
Updated Branches:
refs/heads/master ceeb130c5 -> 91d109d6e
IMPALA-5528: Upgrade GPerfTools to 2.6.3 and tune TCMalloc for KRPC
KRPC in general tends to put more pressure on the thread
caches due to allocations of more small objects (i.e. <1MB).
While some of them are being addressed in KUDU-1865, it's shown
that the following TCMalloc workarounds will provide reasonable
performance with KRPC:
- TCMALLOC_TRANSFER_NUM_OBJ:
- maximum number of object per classe type to transfer between
thread and central caches.
- the default value of 512 in 2.5.2 seems to cause the spin lock
in the central cache to be held for too long with KRPC. 2.6.0
and latter reverts this value to 32 by default.
- TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES
- total amount of memory allocated to all thread caches in bytes
- the default value is 32MB. We need to bump it to 1GB which is the
internal cap in TCMalloc.
This change upgrades GPerfTools/TCMalloc to 2.6.3 to pick up the
change of the default value of TCMALLOC_TRANSFER_NUM_OBJ.
In addition, when KRPC is enabled and FLAGS_TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES
has the default value of 0, we will automatically bump the thread cache sizes
to 1GB. Without these workarounds, stress test with KRPC will grind to a halt
due to contention for the spinlock in TCMalloc's central cache. With these
workarounds, the stress test completes within the same ballpark as thrift.
Also did a perf run with Thrift. The regression in TPCH-Q2 is mostly due to sensitivity
in runtime filter timing and the avg can be dragged up due to a bad run when filters
arrive late. No regression as measured in targeted-perf.
+------------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+------------+-----------------------+---------+------------+------------+----------------+
| TPCH(_300) | parquet / none / none | 18.93 | -0.84% | 10.08 | +1.45% |
+------------+-----------------------+---------+------------+------------+----------------+
+------------+----------+-----------------------+--------+-------------+------------+------------+----------------+-------------+-------+
| Workload | Query | File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters |
+------------+----------+-----------------------+--------+-------------+------------+------------+----------------+-------------+-------+
| TPCH(_300) | TPCH-Q2 | parquet / none / none | 6.28 | 3.25 | R +93.41% | * 49.77% * | * 12.47% * | 1 | 3 |
| TPCH(_300) | TPCH-Q4 | parquet / none / none | 5.00 | 4.77 | +4.83% | 0.41% | 0.03% | 1 | 3 |
| TPCH(_300) | TPCH-Q13 | parquet / none / none | 21.29 | 20.69 | +2.90% | 0.55% | 0.37% | 1 | 3 |
| TPCH(_300) | TPCH-Q11 | parquet / none / none | 1.73 | 1.71 | +0.94% | 1.69% | 2.85% | 1 | 3 |
| TPCH(_300) | TPCH-Q14 | parquet / none / none | 6.03 | 5.99 | +0.76% | 0.00% | 0.95% | 1 | 3 |
| TPCH(_300) | TPCH-Q16 | parquet / none / none | 6.97 | 6.93 | +0.58% | 0.74% | 0.73% | 1 | 3 |
| TPCH(_300) | TPCH-Q3 | parquet / none / none | 29.15 | 29.03 | +0.40% | 1.63% | 1.39% | 1 | 3 |
| TPCH(_300) | TPCH-Q1 | parquet / none / none | 14.01 | 13.96 | +0.34% | 1.28% | 0.51% | 1 | 3 |
| TPCH(_300) | TPCH-Q6 | parquet / none / none | 1.27 | 1.27 | -0.03% | 3.69% | 0.07% | 1 | 3 |
| TPCH(_300) | TPCH-Q9 | parquet / none / none | 30.99 | 31.13 | -0.45% | 0.54% | 0.19% | 1 | 3 |
| TPCH(_300) | TPCH-Q5 | parquet / none / none | 48.03 | 48.33 | -0.63% | 4.72% | 0.11% | 1 | 3 |
| TPCH(_300) | TPCH-Q7 | parquet / none / none | 46.85 | 47.41 | -1.18% | 1.59% | 0.46% | 1 | 3 |
| TPCH(_300) | TPCH-Q8 | parquet / none / none | 7.92 | 8.03 | -1.39% | 3.67% | 5.63% | 1 | 3 |
| TPCH(_300) | TPCH-Q19 | parquet / none / none | 30.98 | 31.51 | -1.67% | 1.33% | 0.82% | 1 | 3 |
| TPCH(_300) | TPCH-Q18 | parquet / none / none | 33.55 | 34.13 | -1.71% | 1.15% | 1.46% | 1 | 3 |
| TPCH(_300) | TPCH-Q10 | parquet / none / none | 9.46 | 9.64 | -1.82% | 0.63% | 0.75% | 1 | 3 |
| TPCH(_300) | TPCH-Q22 | parquet / none / none | 6.00 | 6.16 | -2.58% | 0.08% | 5.12% | 1 | 3 |
| TPCH(_300) | TPCH-Q15 | parquet / none / none | 3.41 | 3.50 | -2.60% | 1.40% | 0.46% | 1 | 3 |
| TPCH(_300) | TPCH-Q12 | parquet / none / none | 3.24 | 3.33 | -2.86% | 1.36% | 1.55% | 1 | 3 |
| TPCH(_300) | TPCH-Q17 | parquet / none / none | 4.65 | 4.83 | -3.58% | 1.17% | 0.42% | 1 | 3 |
| TPCH(_300) | TPCH-Q21 | parquet / none / none | 96.15 | 100.63 | -4.45% | 0.29% | 3.18% | 1 | 3 |
| TPCH(_300) | TPCH-Q20 | parquet / none / none | 3.40 | 3.64 | -6.63% | 4.82% | * 12.70% * | 1 | 3 |
+------------+----------+-----------------------+--------+-------------+------------+------------+----------------+-------------+-------+
+---------------------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+---------------------+-----------------------+---------+------------+------------+----------------+
| TARGETED-PERF(_300) | parquet / none / none | 59.31 | -1.40% | 8.80 | -2.24% |
+---------------------+-----------------------+---------+------------+------------+----------------+
+---------------------+--------------------------------------------------------+-----------------------+---------+-------------+------------+------------+----------------+-------------+-------+
| Workload | Query | File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters |
+---------------------+--------------------------------------------------------+-----------------------+---------+-------------+------------+------------+----------------+-------------+-------+
| TARGETED-PERF(_300) | primitive_conjunct_ordering_2 | parquet / none / none | 36.27 | 30.52 | +18.87% | * 17.02% * | 2.42% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_broadcast_join_1 | parquet / none / none | 1.17 | 1.02 | +14.59% | * 12.82% * | 0.02% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_bigint_in_list | parquet / none / none | 1.03 | 0.92 | +11.54% | 2.51% | 2.53% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_bigint_selective | parquet / none / none | 0.37 | 0.34 | +6.93% | 7.94% | 1.32% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_string_selective | parquet / none / none | 0.47 | 0.44 | +5.97% | 6.11% | 1.12% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_groupby_bigint_highndv | parquet / none / none | 24.35 | 23.87 | +1.99% | 0.82% | 0.63% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_many_fragments | parquet / none / none | 61.64 | 60.93 | +1.17% | 3.25% | 1.07% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_top-n_all | parquet / none / none | 36.63 | 36.31 | +0.87% | 0.35% | 4.62% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_conjunct_ordering_4 | parquet / none / none | 0.87 | 0.86 | +0.66% | 0.47% | 0.03% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_long_predicate | parquet / none / none | 28.83 | 28.69 | +0.49% | 0.24% | 0.10% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_topn_bigint | parquet / none / none | 5.53 | 5.51 | +0.34% | 2.23% | 0.07% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_broadcast_join_3 | parquet / none / none | 58.41 | 58.27 | +0.24% | 2.51% | 0.02% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_decimal_arithmetic | parquet / none / none | 96.67 | 96.59 | +0.09% | 0.41% | 0.08% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_count_star | parquet / none / none | 0.09 | 0.09 | +0.03% | 0.72% | 0.73% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_groupby_decimal_lowndv.test | parquet / none / none | 3.26 | 3.26 | -0.00% | 0.09% | 1.57% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_broadcast_join_2 | parquet / none / none | 4.40 | 4.41 | -0.10% | 0.01% | 1.24% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_shuffle_join_union_all_with_groupby | parquet / none / none | 67.43 | 67.58 | -0.21% | 0.31% | 0.38% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_intrinsic_appx_median | parquet / none / none | 35.14 | 35.27 | -0.34% | 0.39% | 0.47% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_groupby_decimal_highndv | parquet / none / none | 25.94 | 26.07 | -0.51% | 5.33% | 2.30% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_exchange_broadcast | parquet / none / none | 76.54 | 76.96 | -0.54% | 4.50% | 4.70% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_string_like | parquet / none / none | 5.52 | 5.56 | -0.68% | 0.92% | 3.17% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_many_independent_fragments | parquet / none / none | 254.76 | 256.50 | -0.68% | 5.95% | 2.19% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_shuffle_join_one_to_many_string_with_groupby | parquet / none / none | 228.43 | 230.08 | -0.72% | 0.62% | 1.34% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_empty_build_join_1 | parquet / none / none | 1.90 | 1.92 | -1.26% | 1.19% | 2.74% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_exchange_shuffle | parquet / none / none | 78.99 | 80.26 | -1.59% | 0.75% | 1.61% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_shuffle_1mb_rows | parquet / none / none | 1008.91 | 1027.39 | -1.80% | 2.33% | 0.72% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_groupby_bigint_pk | parquet / none / none | 96.58 | 98.62 | -2.07% | 1.08% | 1.98% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_groupby_bigint_lowndv | parquet / none / none | 3.26 | 3.33 | -2.10% | 3.00% | 0.06% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_small_join_1 | parquet / none / none | 0.42 | 0.43 | -2.54% | 0.23% | 1.54% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_string_non_selective | parquet / none / none | 0.90 | 0.93 | -2.54% | 0.18% | 2.45% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_intrinsic_to_date | parquet / none / none | 77.56 | 79.81 | -2.82% | 0.39% | 2.79% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_decimal_non_selective | parquet / none / none | 0.80 | 0.83 | -3.56% | 0.12% | 2.68% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_bigint_non_selective | parquet / none / none | 1.00 | 1.05 | -4.60% | 0.31% | 5.18% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_conjunct_ordering_1 | parquet / none / none | 4.91 | 5.16 | -4.89% | 0.44% | 0.41% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_orderby_all | parquet / none / none | 54.67 | 58.30 | -6.23% | 0.45% | 0.81% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_conjunct_ordering_5 | parquet / none / none | 11.91 | 12.70 | -6.24% | 1.04% | 0.53% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_decimal_selective | parquet / none / none | 0.86 | 0.94 | -8.58% | * 24.14% * | * 36.04% * | 1 | 3 |
| TARGETED-PERF(_300) | primitive_orderby_bigint | parquet / none / none | 15.06 | 16.57 | -9.14% | 3.50% | 0.10% | 1 | 3 |
| TARGETED-PERF(_300) | primitive_filter_in_predicate | parquet / none / none | 1.11 | 1.24 | -10.09% | * 11.28% * | * 12.71% * | 1 | 3 |
| TARGETED-PERF(_300) | primitive_orderby_bigint_expression | parquet / none / none | 18.16 | 24.86 | I -26.97% | 0.96% | * 20.73% * | 1 | 3 |
| TARGETED-PERF(_300) | primitive_conjunct_ordering_3 | parquet / none / none | 0.94 | 1.71 | I -44.74% | 2.68% | * 42.73% * | 1 | 3 |
+---------------------+--------------------------------------------------------+-----------------------+---------+-------------+------------+------------+----------------+-------------+-------+
Change-Id: I5be574435af51fb7a875b16888cca260b341190e
Reviewed-on: http://gerrit.cloudera.org:8080/8991
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/df3a440f
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/df3a440f
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/df3a440f
Branch: refs/heads/master
Commit: df3a440fff38225a03879955c99a87d8ced3b13a
Parents: ceeb130
Author: Michael Ho <kw...@cloudera.com>
Authored: Thu Jan 11 18:12:15 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Jan 12 22:35:13 2018 +0000
----------------------------------------------------------------------
.../runtime/bufferpool/buffer-allocator-test.cc | 7 +++++++
be/src/runtime/bufferpool/free-list-test.cc | 13 +++++++++++-
be/src/runtime/bufferpool/suballocator-test.cc | 13 +++++++++++-
be/src/runtime/exec-env.cc | 21 ++++++++++----------
bin/impala-config.sh | 4 ++--
5 files changed, 44 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/df3a440f/be/src/runtime/bufferpool/buffer-allocator-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/buffer-allocator-test.cc b/be/src/runtime/bufferpool/buffer-allocator-test.cc
index 21a9c08..2b87278 100644
--- a/be/src/runtime/bufferpool/buffer-allocator-test.cc
+++ b/be/src/runtime/bufferpool/buffer-allocator-test.cc
@@ -22,6 +22,8 @@
#include "runtime/bufferpool/buffer-pool-internal.h"
#include "runtime/bufferpool/buffer-pool.h"
#include "runtime/bufferpool/system-allocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
#include "testutil/cpu-util.h"
#include "testutil/gtest-util.h"
#include "util/cpu-info.h"
@@ -39,6 +41,8 @@ using BufferHandle = BufferPool::BufferHandle;
class BufferAllocatorTest : public ::testing::Test {
public:
virtual void SetUp() {
+ test_env_.reset(new TestEnv);
+ ASSERT_OK(test_env_->Init());
dummy_pool_ = obj_pool_.Add(new BufferPool(1, 0, 0));
dummy_reservation_.InitRootTracker(nullptr, 0);
ASSERT_OK(dummy_pool_->RegisterClient("", nullptr, &dummy_reservation_, nullptr, 0,
@@ -59,6 +63,8 @@ class BufferAllocatorTest : public ::testing::Test {
/// The minimum buffer size used in most tests.
const static int64_t TEST_BUFFER_LEN = 1024;
+ boost::scoped_ptr<TestEnv> test_env_;
+
ObjectPool obj_pool_;
/// Need a dummy pool and client to pass around. We bypass the reservation mechanisms
@@ -200,6 +206,7 @@ TEST_F(SystemAllocatorTest, LargeAllocFailure) {
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+ impala::InitFeSupport();
int result = 0;
for (bool mmap : {false, true}) {
for (bool madvise : {false, true}) {
http://git-wip-us.apache.org/repos/asf/impala/blob/df3a440f/be/src/runtime/bufferpool/free-list-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/free-list-test.cc b/be/src/runtime/bufferpool/free-list-test.cc
index 7cb80b4..d3c4c9a 100644
--- a/be/src/runtime/bufferpool/free-list-test.cc
+++ b/be/src/runtime/bufferpool/free-list-test.cc
@@ -21,6 +21,8 @@
#include "common/object-pool.h"
#include "runtime/bufferpool/free-list.h"
#include "runtime/bufferpool/system-allocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
#include "testutil/gtest-util.h"
#include "testutil/rand-util.h"
@@ -31,6 +33,8 @@ namespace impala {
class FreeListTest : public ::testing::Test {
protected:
virtual void SetUp() override {
+ test_env_.reset(new TestEnv);
+ ASSERT_OK(test_env_->Init());
allocator_ = obj_pool_.Add(new SystemAllocator(MIN_BUFFER_LEN));
RandTestUtil::SeedRng("FREE_LIST_TEST_SEED", &rng_);
}
@@ -71,6 +75,8 @@ class FreeListTest : public ::testing::Test {
const static int MIN_BUFFER_LEN = 1024;
+ boost::scoped_ptr<TestEnv> test_env_;
+
/// Per-test random number generator. Seeded before every test.
std::mt19937 rng_;
@@ -157,4 +163,9 @@ TEST_F(FreeListTest, ReturnOrder) {
}
}
-IMPALA_TEST_MAIN();
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+ impala::InitFeSupport();
+ return RUN_ALL_TESTS();
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/df3a440f/be/src/runtime/bufferpool/suballocator-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/suballocator-test.cc b/be/src/runtime/bufferpool/suballocator-test.cc
index 6cd53fb..470b065 100644
--- a/be/src/runtime/bufferpool/suballocator-test.cc
+++ b/be/src/runtime/bufferpool/suballocator-test.cc
@@ -27,6 +27,8 @@
#include "common/object-pool.h"
#include "runtime/bufferpool/reservation-tracker.h"
#include "runtime/bufferpool/suballocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
#include "testutil/death-test-util.h"
#include "testutil/gtest-util.h"
#include "testutil/rand-util.h"
@@ -44,6 +46,8 @@ namespace impala {
class SuballocatorTest : public ::testing::Test {
public:
virtual void SetUp() override {
+ test_env_.reset(new TestEnv);
+ ASSERT_OK(test_env_->Init());
RandTestUtil::SeedRng("SUBALLOCATOR_TEST_SEED", &rng_);
profile_ = RuntimeProfile::Create(&obj_pool_, "test profile");
}
@@ -111,6 +115,8 @@ class SuballocatorTest : public ::testing::Test {
/// Clients for the buffer pool. Deregistered and freed after every test.
vector<unique_ptr<BufferPool::ClientHandle>> clients_;
+ boost::scoped_ptr<TestEnv> test_env_;
+
/// Global profile - recreated for every test.
RuntimeProfile* profile_;
@@ -362,4 +368,9 @@ void SuballocatorTest::AssertMemoryValid(
}
}
-IMPALA_TEST_MAIN();
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+ impala::InitFeSupport();
+ return RUN_ALL_TESTS();
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/df3a440f/be/src/runtime/exec-env.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 6d9a857..94ca834 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -271,16 +271,6 @@ Status ExecEnv::Init() {
"bytes value or percentage: $0", FLAGS_mem_limit));
}
-#if !defined(ADDRESS_SANITIZER) && !defined(THREAD_SANITIZER)
- // Aggressive decommit is required so that unused pages in the TCMalloc page heap are
- // not backed by physical pages and do not contribute towards memory consumption.
- // Enable it in TCMalloc before InitBufferPool().
- if (!MallocExtension::instance()->SetNumericProperty(
- "tcmalloc.aggressive_memory_decommit", 1)) {
- return Status("Failed to enable TCMalloc aggressive decommit.");
- }
-#endif
-
if (!BitUtil::IsPowerOf2(FLAGS_min_buffer_size)) {
return Status(Substitute(
"--min_buffer_size must be a power-of-two: $0", FLAGS_min_buffer_size));
@@ -320,6 +310,11 @@ Status ExecEnv::Init() {
FLAGS_datastream_service_num_svc_threads : CpuInfo::num_cores();
RETURN_IF_ERROR(rpc_mgr_->RegisterService(num_svc_threads,
FLAGS_datastream_service_queue_depth, move(data_svc)));
+ // Bump thread cache to 1GB to reduce contention for TCMalloc central
+ // list's spinlock.
+ if (FLAGS_tcmalloc_max_total_thread_cache_bytes == 0) {
+ FLAGS_tcmalloc_max_total_thread_cache_bytes = 1 << 30;
+ }
}
mem_tracker_.reset(
@@ -432,6 +427,12 @@ Status ExecEnv::StartKrpcService() {
void ExecEnv::InitBufferPool(int64_t min_buffer_size, int64_t capacity,
int64_t clean_pages_limit) {
+#if !defined(ADDRESS_SANITIZER) && !defined(THREAD_SANITIZER)
+ // Aggressive decommit is required so that unused pages in the TCMalloc page heap are
+ // not backed by physical pages and do not contribute towards memory consumption.
+ MallocExtension::instance()->SetNumericProperty(
+ "tcmalloc.aggressive_memory_decommit", 1);
+#endif
buffer_pool_.reset(new BufferPool(min_buffer_size, capacity, clean_pages_limit));
buffer_reservation_.reset(new ReservationTracker());
buffer_reservation_->InitRootTracker(nullptr, capacity);
http://git-wip-us.apache.org/repos/asf/impala/blob/df3a440f/bin/impala-config.sh
----------------------------------------------------------------------
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 058dc01..f5b67a9 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -72,7 +72,7 @@ fi
# moving to a different build of the toolchain, e.g. when a version is bumped or a
# compile option is changed. The build id can be found in the output of the toolchain
# build jobs, it is constructed from the build number and toolchain git hash prefix.
-export IMPALA_TOOLCHAIN_BUILD_ID=482-c2361403fc
+export IMPALA_TOOLCHAIN_BUILD_ID=4-7847dc86c4
# Versions of toolchain dependencies.
# -----------------------------------
export IMPALA_AVRO_VERSION=1.7.4-p4
@@ -99,7 +99,7 @@ export IMPALA_GFLAGS_VERSION=2.2.0-p1
unset IMPALA_GFLAGS_URL
export IMPALA_GLOG_VERSION=0.3.4-p2
unset IMPALA_GLOG_URL
-export IMPALA_GPERFTOOLS_VERSION=2.5
+export IMPALA_GPERFTOOLS_VERSION=2.6.3
unset IMPALA_GPERFTOOLS_URL
export IMPALA_GTEST_VERSION=1.6.0
unset IMPALA_GTEST_URL
[3/4] impala git commit: IMPALA-6363: avoid cscope build races
Posted by jr...@apache.org.
IMPALA-6363: avoid cscope build races
Use the -ignore_readdir_race flag for find so that find doesn't fail if
a directory disappears under it. From what I could tell the flag has
been in GNU find for a long time and is also available in other OS
flavours like BSD and OS X.
Make the step depend on gen-deps so that it can index thrift, protobuf,
etc, output.
Change-Id: I22bdb7c64036cb88a8a10907af35c5e3a55a9195
Reviewed-on: http://gerrit.cloudera.org:8080/9007
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/6bff0bd7
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/6bff0bd7
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/6bff0bd7
Branch: refs/heads/master
Commit: 6bff0bd766ece21c6f7605f0307fcc710641ab7f
Parents: 10fb24a
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu Jan 11 11:28:52 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Jan 13 03:24:06 2018 +0000
----------------------------------------------------------------------
CMakeLists.txt | 2 +-
bin/gen-cscope.sh | 7 +++++--
2 files changed, 6 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/6bff0bd7/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 74cf37e..37a6324 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -368,7 +368,7 @@ add_custom_target(shell_tarball DEPENDS gen-deps
COMMAND "${CMAKE_SOURCE_DIR}/shell/make_shell_tarball.sh"
)
-add_custom_target(cscope ALL
+add_custom_target(cscope ALL DEPENDS gen-deps
COMMAND "${CMAKE_SOURCE_DIR}/bin/gen-cscope.sh"
)
http://git-wip-us.apache.org/repos/asf/impala/blob/6bff0bd7/bin/gen-cscope.sh
----------------------------------------------------------------------
diff --git a/bin/gen-cscope.sh b/bin/gen-cscope.sh
index 0007782..d3b4423 100755
--- a/bin/gen-cscope.sh
+++ b/bin/gen-cscope.sh
@@ -22,7 +22,10 @@ bin=`dirname "$0"`
bin=`cd "$bin"; pwd`
. "$bin"/impala-config.sh
-# Generate list of files for Cscope to index
+# Generate list of files for Cscope to index.
+# -ignore_readdir_race: this scripts runs in parallel with other build steps, so races
+# with unrelated directories being deleted are possible (IMPALA-6363).
cd $IMPALA_HOME
-find . -regex '.*\.\(cc\|c\|hh\|h\|java\|thrift\|flex\|y\)$' > cscope.files
+find . -ignore_readdir_race -regex '.*\.\(cc\|c\|hh\|h\|java\|thrift\|flex\|y\)$' \
+ > cscope.files
[4/4] impala git commit: IMPALA-6307: CTAS statement fails with
duplicate column exception.
Posted by jr...@apache.org.
IMPALA-6307: CTAS statement fails with duplicate column exception.
A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.
IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.
Testing:
* Regression test added to AnalyzeDDLTest.
* Exhaustive Jenkins build and test.
Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Reviewed-on: http://gerrit.cloudera.org:8080/8930
Reviewed-by: Dimitris Tsirogiannis <dt...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/91d109d6
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/91d109d6
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/91d109d6
Branch: refs/heads/master
Commit: 91d109d6e982b83500c7869f866ec719f73eb7b6
Parents: 6bff0bd
Author: Zoram Thanga <zo...@cloudera.com>
Authored: Tue Jan 2 11:02:17 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Jan 13 03:44:43 2018 +0000
----------------------------------------------------------------------
.../org/apache/impala/analysis/AnalysisContext.java | 3 ++-
.../apache/impala/analysis/CreateTableAsSelectStmt.java | 9 ++++++---
.../org/apache/impala/analysis/TableDataLayout.java | 5 +++++
.../main/java/org/apache/impala/analysis/TableDef.java | 3 ++-
.../java/org/apache/impala/analysis/AnalyzeDDLTest.java | 12 ++++++++++++
5 files changed, 27 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/91d109d6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index 53aa000..71a185e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -404,7 +404,8 @@ public class AnalysisContext {
if (reAnalyze) {
// The rewrites should have no user-visible effect. Remember the original result
// types and column labels to restore them after the rewritten stmt has been
- // reset() and re-analyzed.
+ // reset() and re-analyzed. For a CTAS statement, the types represent column types
+ // of the table that will be created, including the partition columns, if any.
List<Type> origResultTypes = Lists.newArrayList();
for (Expr e: analysisResult_.stmt_.getResultExprs()) {
origResultTypes.add(e.getType());
http://git-wip-us.apache.org/repos/asf/impala/blob/91d109d6/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
index 842083e..ba9a02a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
@@ -158,7 +158,6 @@ public class CreateTableAsSelectStmt extends StatementBase {
// Add the columns from the select statement to the create statement.
int colCnt = tmpQueryStmt.getColLabels().size();
- createStmt_.getColumnDefs().clear();
for (int i = 0; i < colCnt; ++i) {
ColumnDef colDef = new ColumnDef(tmpQueryStmt.getColLabels().get(i), null,
Collections.<ColumnDef.Option, Object>emptyMap());
@@ -220,8 +219,12 @@ public class CreateTableAsSelectStmt extends StatementBase {
super.castResultExprs(types);
// Set types of column definitions.
List<ColumnDef> colDefs = createStmt_.getColumnDefs();
- Preconditions.checkState(colDefs.size() == types.size());
- for (int i = 0; i < types.size(); ++i) colDefs.get(i).setType(types.get(i));
+ List<ColumnDef> partitionColDefs = createStmt_.getPartitionColumnDefs();
+ Preconditions.checkState(colDefs.size() + partitionColDefs.size() == types.size());
+ for (int i = 0; i < colDefs.size(); ++i) colDefs.get(i).setType(types.get(i));
+ for (int i = 0; i < partitionColDefs.size(); ++i) {
+ partitionColDefs.get(i).setType(types.get(i + colDefs.size()));
+ }
}
@Override
http://git-wip-us.apache.org/repos/asf/impala/blob/91d109d6/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java b/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
index aef5732..fea809d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
@@ -52,4 +52,9 @@ class TableDataLayout {
List<ColumnDef> getPartitionColumnDefs() { return partitionColDefs_; }
List<KuduPartitionParam> getKuduPartitionParams() { return kuduPartitionParams_; }
+
+ public void reset() {
+ partitionColDefs_.clear();
+ kuduPartitionParams_.clear();
+ }
}
http://git-wip-us.apache.org/repos/asf/impala/blob/91d109d6/fe/src/main/java/org/apache/impala/analysis/TableDef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TableDef.java b/fe/src/main/java/org/apache/impala/analysis/TableDef.java
index f4901f9..178a976 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TableDef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TableDef.java
@@ -35,7 +35,6 @@ import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.THdfsFileFormat;
import org.apache.impala.util.MetaStoreUtil;
-import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
@@ -158,6 +157,8 @@ class TableDef {
public void reset() {
primaryKeyColDefs_.clear();
+ dataLayout_.reset();
+ columnDefs_.clear();
isAnalyzed_ = false;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/91d109d6/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
index 09f5a73..53ee930 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -1507,6 +1507,12 @@ public class AnalyzeDDLTest extends FrontendTestBase {
"as select count(*) as CNT from functional.alltypes");
AnalyzesOk("create table functional.tbl as select a.* from functional.alltypes a " +
"join functional.alltypes b on (a.int_col=b.int_col) limit 1000");
+ // CTAS with a select query that requires expression rewrite (IMPALA-6307)
+ AnalyzesOk("create table functional.ctas_tbl partitioned by (year) as " +
+ "with tmp as (select a.timestamp_col, a.year from functional.alltypes a " +
+ "left join functional.alltypes b " +
+ "on b.timestamp_col between a.timestamp_col and a.timestamp_col) " +
+ "select a.timestamp_col, a.year from tmp a");
// Caching operations
AnalyzesOk("create table functional.newtbl cached in 'testPool'" +
@@ -1606,6 +1612,12 @@ public class AnalyzeDDLTest extends FrontendTestBase {
// IMPALA-5796: CTAS into a Kudu table with expr rewriting.
AnalyzesOk("create table t primary key(id) stored as kudu as select id, bool_col " +
"from functional.alltypestiny where id between 0 and 10");
+ // CTAS with a select query that requires expression rewrite (IMPALA-6307)
+ AnalyzesOk("create table t primary key(year) stored as kudu as " +
+ "with tmp as (select a.timestamp_col, a.year from functional.alltypes a " +
+ "left join functional.alltypes b " +
+ "on b.timestamp_col between a.timestamp_col and a.timestamp_col) " +
+ "select a.timestamp_col, a.year from tmp a");
// CTAS in an external Kudu table
AnalysisError("create external table t stored as kudu " +
"tblproperties('kudu.table_name'='t') as select id, int_col from " +
[2/4] impala git commit: IMPALA-6383: free memory after skipping
parquet row groups
Posted by jr...@apache.org.
IMPALA-6383: free memory after skipping parquet row groups
Before this patch, resources were only flushed after breaking out of
NextRowGroup(). This is a problem because resources can be allocated
for skipped row groups (e.g. for reading dictionaries).
Testing:
Tested in conjunction with a prototype buffer pool patch that was
DCHECKing before the change.
Added DCHECKs to the current version to ensure the streams are cleared
up as expected.
Change-Id: Ibc2f8f27c9b238be60261539f8d4be2facb57a2b
Reviewed-on: http://gerrit.cloudera.org:8080/9002
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/10fb24af
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/10fb24af
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/10fb24af
Branch: refs/heads/master
Commit: 10fb24afb966c567adcf632a314f6af1826f19fc
Parents: df3a440
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Jan 10 15:35:41 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Jan 13 02:48:08 2018 +0000
----------------------------------------------------------------------
be/src/exec/hdfs-parquet-scanner.cc | 26 +++++++++++++++++++++-----
be/src/exec/hdfs-parquet-scanner.h | 5 +++++
be/src/exec/scanner-context.h | 8 ++++----
3 files changed, 30 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/10fb24af/be/src/exec/hdfs-parquet-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner.cc b/be/src/exec/hdfs-parquet-scanner.cc
index f0f280d..6380722 100644
--- a/be/src/exec/hdfs-parquet-scanner.cc
+++ b/be/src/exec/hdfs-parquet-scanner.cc
@@ -228,6 +228,7 @@ Status HdfsParquetScanner::Open(ScannerContext* context) {
// Release I/O buffers immediately to make sure they are cleaned up
// in case we return a non-OK status anywhere below.
context_->ReleaseCompletedResources(true);
+ context_->ClearStreams();
RETURN_IF_ERROR(footer_status);
// Parse the file schema into an internal representation for schema resolution.
@@ -263,7 +264,7 @@ void HdfsParquetScanner::Close(RowBatch* row_batch) {
}
} else {
template_tuple_pool_->FreeAll();
- dictionary_pool_.get()->FreeAll();
+ dictionary_pool_->FreeAll();
context_->ReleaseCompletedResources(true);
for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(nullptr);
// The scratch batch may still contain tuple data. We can get into this case if
@@ -478,7 +479,6 @@ Status HdfsParquetScanner::GetNextInternal(RowBatch* row_batch) {
// Transfer resources and clear streams if there is any leftover from the previous
// row group. We will create new streams for the next row group.
FlushRowGroupResources(row_batch);
- context_->ClearStreams();
if (!advance_row_group_) {
Status status =
ValidateEndOfRowGroup(column_readers_, row_group_idx_, row_group_rows_read_);
@@ -619,6 +619,9 @@ Status HdfsParquetScanner::NextRowGroup() {
while (true) {
// Reset the parse status for the next row group.
parse_status_ = Status::OK();
+ // Make sure that we don't have leftover resources from the file metadata scan range
+ // or previous row groups.
+ DCHECK_EQ(0, context_->NumStreams());
++row_group_idx_;
if (row_group_idx_ >= file_metadata_.row_groups.size()) {
@@ -669,6 +672,9 @@ Status HdfsParquetScanner::NextRowGroup() {
// of the column.
RETURN_IF_ERROR(InitColumns(row_group_idx_, dict_filterable_readers_));
+ // InitColumns() may have allocated resources to scan columns. If we skip this row
+ // group below, we must call ReleaseSkippedRowGroupResources() before continuing.
+
// If there is a dictionary-encoded column where every value is eliminated
// by a conjunct, the row group can be eliminated. This initializes dictionaries
// for all columns visited.
@@ -677,10 +683,12 @@ Status HdfsParquetScanner::NextRowGroup() {
if (!status.ok()) {
// Either return an error or skip this row group if it is ok to ignore errors
RETURN_IF_ERROR(state_->LogOrReturnError(status.msg()));
+ ReleaseSkippedRowGroupResources();
continue;
}
if (skip_row_group_on_dict_filters) {
COUNTER_ADD(num_dict_filtered_row_groups_counter_, 1);
+ ReleaseSkippedRowGroupResources();
continue;
}
@@ -692,6 +700,7 @@ Status HdfsParquetScanner::NextRowGroup() {
if (!status.ok()) {
// Either return an error or skip this row group if it is ok to ignore errors
RETURN_IF_ERROR(state_->LogOrReturnError(status.msg()));
+ ReleaseSkippedRowGroupResources();
continue;
}
@@ -730,9 +739,16 @@ void HdfsParquetScanner::FlushRowGroupResources(RowBatch* row_batch) {
row_batch->tuple_data_pool()->AcquireData(dictionary_pool_.get(), false);
scratch_batch_->ReleaseResources(row_batch->tuple_data_pool());
context_->ReleaseCompletedResources(true);
- for (ParquetColumnReader* col_reader : column_readers_) {
- col_reader->Close(row_batch);
- }
+ for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(row_batch);
+ context_->ClearStreams();
+}
+
+void HdfsParquetScanner::ReleaseSkippedRowGroupResources() {
+ dictionary_pool_->FreeAll();
+ scratch_batch_->ReleaseResources(nullptr);
+ context_->ReleaseCompletedResources(true);
+ for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(nullptr);
+ context_->ClearStreams();
}
bool HdfsParquetScanner::IsDictFilterable(ParquetColumnReader* col_reader) {
http://git-wip-us.apache.org/repos/asf/impala/blob/10fb24af/be/src/exec/hdfs-parquet-scanner.h
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner.h b/be/src/exec/hdfs-parquet-scanner.h
index 99b5a60..cea06ed 100644
--- a/be/src/exec/hdfs-parquet-scanner.h
+++ b/be/src/exec/hdfs-parquet-scanner.h
@@ -642,6 +642,11 @@ class HdfsParquetScanner : public HdfsScanner {
/// Should be called after completing a row group and when returning the last batch.
void FlushRowGroupResources(RowBatch* row_batch);
+ /// Releases resources associated with a row group that was skipped and closes all
+ /// column readers. Should be called after skipping a row group from which no rows
+ /// were returned.
+ void ReleaseSkippedRowGroupResources();
+
/// Evaluates whether the column reader is eligible for dictionary predicates
bool IsDictFilterable(ParquetColumnReader* col_reader);
http://git-wip-us.apache.org/repos/asf/impala/blob/10fb24af/be/src/exec/scanner-context.h
----------------------------------------------------------------------
diff --git a/be/src/exec/scanner-context.h b/be/src/exec/scanner-context.h
index e316063..09a4bdc 100644
--- a/be/src/exec/scanner-context.h
+++ b/be/src/exec/scanner-context.h
@@ -89,7 +89,6 @@ class ScannerContext {
ScannerContext(RuntimeState*, HdfsScanNodeBase*, HdfsPartitionDescriptor*,
io::ScanRange* scan_range, const std::vector<FilterContext>& filter_ctxs,
MemPool* expr_results_pool);
-
/// Destructor verifies that all stream objects have been released.
~ScannerContext();
@@ -338,6 +337,8 @@ class ScannerContext {
return streams_[idx].get();
}
+ int NumStreams() const { return streams_.size(); }
+
/// Release completed resources for all streams, e.g. the last buffer in each stream if
/// the current read position is at the end of the buffer. If 'done' is true all
/// resources are freed, even if the caller has not read that data yet. After calling
@@ -354,8 +355,8 @@ class ScannerContext {
/// size to 0.
void ClearStreams();
- /// Add a stream to this ScannerContext for 'range'. Returns the added stream.
- /// The stream is created in the runtime state's object pool
+ /// Add a stream to this ScannerContext for 'range'. The stream is owned by this
+ /// context.
Stream* AddStream(io::ScanRange* range);
/// Returns false if scan_node_ is multi-threaded and has been cancelled.
@@ -370,7 +371,6 @@ class ScannerContext {
RuntimeState* state_;
HdfsScanNodeBase* scan_node_;
-
HdfsPartitionDescriptor* partition_desc_;
/// Vector of streams. Non-columnar formats will always have one stream per context.