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/05/07 16:42:59 UTC
[1/4] impala git commit: IMPALA-6507: remove --disable_mem_pools
debug feature
Repository: impala
Updated Branches:
refs/heads/2.x dd6f28747 -> f55479e07
IMPALA-6507: remove --disable_mem_pools debug feature
Save some maintenance overhead by simplifying memory
allocation code paths. ASAN poisoning provides the same general
functionality and is on by default.
Change-Id: I78062569448fed0d076ec506eb8e097ce44d9fea
Reviewed-on: http://gerrit.cloudera.org:8080/10294
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/10320
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/ab87d1b9
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/ab87d1b9
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/ab87d1b9
Branch: refs/heads/2.x
Commit: ab87d1b96ffcb2273f138cca8b047e760f1a1384
Parents: dd6f287
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu May 3 09:55:24 2018 -0700
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Sat May 5 18:30:28 2018 +0000
----------------------------------------------------------------------
be/src/common/global-flags.cc | 3 +--
be/src/runtime/bufferpool/buffer-allocator.cc | 11 +----------
be/src/runtime/free-pool.h | 12 ------------
be/src/runtime/io/disk-io-mgr.cc | 1 -
be/src/runtime/mem-pool.cc | 17 ++---------------
5 files changed, 4 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/ab87d1b9/be/src/common/global-flags.cc
----------------------------------------------------------------------
diff --git a/be/src/common/global-flags.cc b/be/src/common/global-flags.cc
index 8ea1c90..d8b1bae 100644
--- a/be/src/common/global-flags.cc
+++ b/be/src/common/global-flags.cc
@@ -85,8 +85,7 @@ DEFINE_int64(tcmalloc_max_total_thread_cache_bytes, 0, "(Advanced) Bound on the
DEFINE_bool(abort_on_config_error, true, "Abort Impala startup if there are improper "
"configs or running on unsupported hardware.");
-DEFINE_bool(disable_mem_pools, false, "Set to true to disable memory pooling. "
- "This can be used to help diagnose memory corruption issues.");
+DEFINE_bool_hidden(disable_mem_pools, false, "Removed - has no effect.");
DEFINE_bool(compact_catalog_topic, true, "If true, catalog updates sent via the "
"statestore are compacted before transmission. This saves network bandwidth at the"
http://git-wip-us.apache.org/repos/asf/impala/blob/ab87d1b9/be/src/runtime/bufferpool/buffer-allocator.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/buffer-allocator.cc b/be/src/runtime/bufferpool/buffer-allocator.cc
index 09c2623..aa9e51d 100644
--- a/be/src/runtime/bufferpool/buffer-allocator.cc
+++ b/be/src/runtime/bufferpool/buffer-allocator.cc
@@ -30,8 +30,6 @@
#include "common/names.h"
-DECLARE_bool(disable_mem_pools);
-
namespace impala {
/// Decrease 'bytes_remaining' by up to 'max_decrease', down to a minimum of 0.
@@ -482,12 +480,6 @@ BufferPool::FreeBufferArena::~FreeBufferArena() {
void BufferPool::FreeBufferArena::AddFreeBuffer(BufferHandle&& buffer) {
lock_guard<SpinLock> al(lock_);
- if (FLAGS_disable_mem_pools) {
- int64_t len = buffer.len();
- parent_->system_allocator_->Free(move(buffer));
- parent_->system_bytes_remaining_.Add(len);
- return;
- }
PerSizeLists* lists = GetListsForSize(buffer.len());
lists->AddFreeBuffer(move(buffer));
}
@@ -625,8 +617,7 @@ pair<int64_t, int64_t> BufferPool::FreeBufferArena::FreeSystemMemory(
}
void BufferPool::FreeBufferArena::AddCleanPage(Page* page) {
- bool eviction_needed = FLAGS_disable_mem_pools
- || DecreaseBytesRemaining(
+ bool eviction_needed = DecreaseBytesRemaining(
page->len, true, &parent_->clean_page_bytes_remaining_) == 0;
lock_guard<SpinLock> al(lock_);
PerSizeLists* lists = GetListsForSize(page->len);
http://git-wip-us.apache.org/repos/asf/impala/blob/ab87d1b9/be/src/runtime/free-pool.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/free-pool.h b/be/src/runtime/free-pool.h
index 52c7137..d10f9b9 100644
--- a/be/src/runtime/free-pool.h
+++ b/be/src/runtime/free-pool.h
@@ -30,8 +30,6 @@
#include "runtime/mem-pool.h"
#include "util/bit-util.h"
-DECLARE_bool(disable_mem_pools);
-
namespace impala {
/// Implementation of a free pool to recycle allocations. The pool is broken
@@ -63,9 +61,6 @@ class FreePool {
/// Return a non-nullptr dummy pointer. nullptr is reserved for failures.
if (UNLIKELY(requested_size == 0)) return mem_pool_->EmptyAllocPtr();
++net_allocations_;
- if (FLAGS_disable_mem_pools) {
- return reinterpret_cast<uint8_t*>(malloc(requested_size));
- }
/// MemPool allocations are 8-byte aligned, so making allocations < 8 bytes
/// doesn't save memory and eliminates opportunities to recycle allocations.
int64_t actual_size = std::max<int64_t>(8, requested_size);
@@ -103,10 +98,6 @@ class FreePool {
void Free(uint8_t* ptr) {
if (UNLIKELY(ptr == nullptr || ptr == mem_pool_->EmptyAllocPtr())) return;
--net_allocations_;
- if (FLAGS_disable_mem_pools) {
- free(ptr);
- return;
- }
FreeListNode* node = reinterpret_cast<FreeListNode*>(ptr - sizeof(FreeListNode));
FreeListNode* list = node->list;
#ifndef NDEBUG
@@ -133,9 +124,6 @@ class FreePool {
/// free the memory buffer pointed to by "ptr" in this case.
uint8_t* Reallocate(uint8_t* ptr, int64_t size) {
if (UNLIKELY(ptr == nullptr || ptr == mem_pool_->EmptyAllocPtr())) return Allocate(size);
- if (FLAGS_disable_mem_pools) {
- return reinterpret_cast<uint8_t*>(realloc(reinterpret_cast<void*>(ptr), size));
- }
FreeListNode* node = reinterpret_cast<FreeListNode*>(ptr - sizeof(FreeListNode));
FreeListNode* list = node->list;
#ifndef NDEBUG
http://git-wip-us.apache.org/repos/asf/impala/blob/ab87d1b9/be/src/runtime/io/disk-io-mgr.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/io/disk-io-mgr.cc b/be/src/runtime/io/disk-io-mgr.cc
index 170b47e..28f2293 100644
--- a/be/src/runtime/io/disk-io-mgr.cc
+++ b/be/src/runtime/io/disk-io-mgr.cc
@@ -30,7 +30,6 @@
#include "util/hdfs-util.h"
#include "util/time.h"
-DECLARE_bool(disable_mem_pools);
#ifndef NDEBUG
DECLARE_int32(stress_scratch_write_delay_ms);
#endif
http://git-wip-us.apache.org/repos/asf/impala/blob/ab87d1b9/be/src/runtime/mem-pool.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/mem-pool.cc b/be/src/runtime/mem-pool.cc
index 8f58168..3038185 100644
--- a/be/src/runtime/mem-pool.cc
+++ b/be/src/runtime/mem-pool.cc
@@ -30,8 +30,6 @@ using namespace impala;
#define MEM_POOL_POISON (0x66aa77bb)
-DECLARE_bool(disable_mem_pools);
-
const int MemPool::INITIAL_CHUNK_SIZE;
const int MemPool::MAX_CHUNK_SIZE;
@@ -128,16 +126,8 @@ bool MemPool::FindChunk(int64_t min_size, bool check_limits) noexcept {
// Didn't find a big enough free chunk - need to allocate new chunk.
int64_t chunk_size;
DCHECK_LE(next_chunk_size_, MAX_CHUNK_SIZE);
-
- if (FLAGS_disable_mem_pools) {
- // Disable pooling by sizing the chunk to fit only this allocation.
- // Make sure the alignment guarantees are respected.
- chunk_size = std::max<int64_t>(min_size, alignof(std::max_align_t));
- } else {
- DCHECK_GE(next_chunk_size_, INITIAL_CHUNK_SIZE);
- chunk_size = max<int64_t>(min_size, next_chunk_size_);
- }
-
+ DCHECK_GE(next_chunk_size_, INITIAL_CHUNK_SIZE);
+ chunk_size = max<int64_t>(min_size, next_chunk_size_);
if (check_limits) {
if (!mem_tracker_->TryConsume(chunk_size)) return false;
} else {
@@ -252,9 +242,6 @@ bool MemPool::CheckIntegrity(bool check_current_chunk_empty) {
DCHECK_EQ(zero_length_region_, MEM_POOL_POISON);
DCHECK_LT(current_chunk_idx_, static_cast<int>(chunks_.size()));
- // Without pooling, there are way too many chunks and this takes too long.
- if (FLAGS_disable_mem_pools) return true;
-
// check that current_chunk_idx_ points to the last chunk with allocated data
int64_t total_allocated = 0;
for (int i = 0; i < chunks_.size(); ++i) {
[2/4] impala git commit: IMPALA-6968: Fix TestBlockVerification
flakiness
Posted by ta...@apache.org.
IMPALA-6968: Fix TestBlockVerification flakiness
The bug is that the byte in the encrypted data is '?' around 1/256 runs
of the test. Instead, flip a bit in the original data so that it's
always different from the input.
Change-Id: Ibdf063ff32848035af667c7cd2a1268f5b785cfe
Reviewed-on: http://gerrit.cloudera.org:8080/10301
Reviewed-by: Sailesh Mukil <sa...@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/d1476d1e
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d1476d1e
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d1476d1e
Branch: refs/heads/2.x
Commit: d1476d1e06dded6ca0f78aebc879f7ca34ee685b
Parents: ab87d1b
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu May 3 16:05:53 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat May 5 19:10:18 2018 +0000
----------------------------------------------------------------------
be/src/runtime/tmp-file-mgr-test.cc | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/d1476d1e/be/src/runtime/tmp-file-mgr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc
index 6f82658..9eb197e 100644
--- a/be/src/runtime/tmp-file-mgr-test.cc
+++ b/be/src/runtime/tmp-file-mgr-test.cc
@@ -541,9 +541,12 @@ void TmpFileMgrTest::TestBlockVerification() {
WaitForCallbacks(1);
// Modify the data in the scratch file and check that a read error occurs.
+ LOG(INFO) << "Corrupting " << file_path;
+ uint8_t corrupt_byte = data[0] ^ 1;
FILE* file = fopen(file_path.c_str(), "rb+");
- fputc('?', file);
- fclose(file);
+ ASSERT_TRUE(file != nullptr);
+ ASSERT_EQ(corrupt_byte, fputc(corrupt_byte, file));
+ ASSERT_EQ(0, fclose(file));
vector<uint8_t> tmp;
tmp.resize(data.size());
Status read_status = file_group.Read(handle.get(), MemRange(tmp.data(), tmp.size()));
@@ -552,7 +555,8 @@ void TmpFileMgrTest::TestBlockVerification() {
<< read_status.GetDetail();
// Modify the data in memory. Restoring the data should fail.
- data[0] = '?';
+ LOG(INFO) << "Corrupting data in memory";
+ data[0] = corrupt_byte;
Status restore_status = file_group.RestoreData(move(handle), data_mem_range);
LOG(INFO) << restore_status.GetDetail();
EXPECT_EQ(TErrorCode::SCRATCH_READ_VERIFY_FAILED, restore_status.code())
[3/4] impala git commit: IMPALA-6975:
TestRuntimeRowFilters.test_row_filters failing with Memory limit exceeded
Posted by ta...@apache.org.
IMPALA-6975: TestRuntimeRowFilters.test_row_filters failing with Memory limit exceeded
This test has started failing relatively frequently. We think that
this may be due to timing differences of when RPCs arrive from the
recent changes with KRPC.
Increasing the memory limit should allow this test to pass
consistently.
Change-Id: Ie39482e2a0aee402ce156b11cce51038cff5e61a
Reviewed-on: http://gerrit.cloudera.org:8080/10315
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/f55479e0
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/f55479e0
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/f55479e0
Branch: refs/heads/2.x
Commit: f55479e078b7204a0ad8924160556f2457a46d86
Parents: e727dc0
Author: Sailesh Mukil <sa...@cloudera.com>
Authored: Fri May 4 12:13:20 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat May 5 19:10:18 2018 +0000
----------------------------------------------------------------------
.../functional-query/queries/QueryTest/runtime_row_filters.test | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/f55479e0/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
index fdca977..d16e9c0 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
@@ -312,7 +312,7 @@ from alltypes a join [SHUFFLE] alltypessmall c
SET RUNTIME_FILTER_WAIT_TIME_MS=$RUNTIME_FILTER_WAIT_TIME_MS;
SET RUNTIME_FILTER_MODE=GLOBAL;
-SET MEM_LIMIT=200MB;
+SET MEM_LIMIT=250MB;
select straight_join count(*)
from tpch_parquet.lineitem l1 join tpch_parquet.lineitem l2
on lower(upper(lower(upper(lower(l1.l_comment))))) = concat(l2.l_comment, 'foo')
[4/4] impala git commit: IMPALA-6866: Rework timeouts for
test_exchange_delays.py
Posted by ta...@apache.org.
IMPALA-6866: Rework timeouts for test_exchange_delays.py
Isilon has been failing on the exchange-delays-zero-rows
test case due to slow scans. Running this part of the
test with a larger value for stress_datastream_recvr_delay_ms
solved the issue.
Since this part of the test is sensitive to slow scans,
I pulled this out into its own test. The new test can
apply an extra delay for platforms with slow scans.
This subsumes the fix for IMPALA-6811 and treats S3,
ADLS, and ISILON as platforms with slow scans.
test_exchange_small_delay and test_exchange_large_delay
(minus exchange-delays-zero-rows) go back to the timeouts
they were using before IMPALA-6811. These tests did not
see issues with those timeouts.
The new arrangement with test_exchange_large_delay_zero_rows
does not change any timeouts except for Isilon. This applies
a slow scan extra delay on top of Isilon's already slow
build.
Change-Id: I2e919a4e502b1e6a4156aafbbe4b5ddfe679ed89
Reviewed-on: http://gerrit.cloudera.org:8080/10208
Reviewed-by: Michael Brown <mi...@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/e727dc07
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/e727dc07
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/e727dc07
Branch: refs/heads/2.x
Commit: e727dc07494fba592333d5236c878e2e6ea48987
Parents: d1476d1
Author: Joe McDonnell <jo...@cloudera.com>
Authored: Wed Apr 25 12:44:37 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat May 5 19:10:18 2018 +0000
----------------------------------------------------------------------
tests/custom_cluster/test_exchange_delays.py | 24 ++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/e727dc07/tests/custom_cluster/test_exchange_delays.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_exchange_delays.py b/tests/custom_cluster/test_exchange_delays.py
index 93de102..1d7d14b 100644
--- a/tests/custom_cluster/test_exchange_delays.py
+++ b/tests/custom_cluster/test_exchange_delays.py
@@ -25,8 +25,7 @@ from tests.util.filesystem_utils import IS_S3, IS_ADLS, IS_ISILON
SLOW_BUILD_TIMEOUT=20000
DELAY_MS = specific_build_type_timeout(10000, slow_build_timeout=SLOW_BUILD_TIMEOUT)
# IMPALA-6381: Isilon can behave as a slow build.
-# IMPALA-6811: S3/ADLS can also have a slow scan that requires a longer delay.
-if IS_S3 or IS_ADLS or IS_ISILON:
+if IS_ISILON:
DELAY_MS = SLOW_BUILD_TIMEOUT
@SkipIfBuildType.not_dev_build
@@ -60,9 +59,20 @@ class TestExchangeDelays(CustomClusterTestSuite):
"""
self.run_test_case('QueryTest/exchange-delays', vector)
- # Test the special case when no batches are sent and the EOS message times out.
- # IMPALA-6811: For future reference, the SQL used for this test case requires
- # that the scan complete before the fragment sends the EOS message. A slow scan can
- # cause this test to fail, because the receivers could be set up before the
- # fragment starts sending (and thus can't time out).
+ # The SQL used for test_exchange_large_delay_zero_rows requires that the scan complete
+ # before the fragment sends the EOS message. A slow scan can cause this test to fail,
+ # because the receivers could be set up before the fragment starts sending (and thus
+ # can't time out). Use a longer delay for platforms that have slow scans:
+ # IMPALA-6811: S3/ADLS have slow scans.
+ # IMPALA-6866: Isilon has slow scans (and is counted as a slow build above).
+ SLOW_SCAN_EXTRA_DELAY_MS = 10000
+ if IS_S3 or IS_ADLS or IS_ISILON:
+ DELAY_MS += SLOW_SCAN_EXTRA_DELAY_MS
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ "--stress_datastream_recvr_delay_ms={0}".format(DELAY_MS)
+ + " --datastream_sender_timeout_ms=1")
+ def test_exchange_large_delay_zero_rows(self, vector):
+ """Test the special case when no batches are sent and the EOS message times out."""
self.run_test_case('QueryTest/exchange-delays-zero-rows', vector)