IMPALA-6118: Fix assertion failure in coordinator bloom filter updating Coordinator::UpdateFilter wrongly assumed std::string::capacity() to be 0 after std::string::clear(), resulting in an DCHECK failure in memory tracking. This patch tracks size() instead of capacity() to fix the bug.
Change-Id: Ia28f9eb8d0b3e795d798949a1cfcfeaaec7040ed Reviewed-on: http://gerrit.cloudera.org:8080/8410 Reviewed-by: Tim Armstrong <[email protected]> Reviewed-by: Alex Behm <[email protected]> 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/d1594298 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d1594298 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d1594298 Branch: refs/heads/master Commit: d1594298eac2f3b5b4e9a1ce72de499df2b3ecc1 Parents: 471285b Author: Tianyi Wang <[email protected]> Authored: Fri Oct 27 11:02:40 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Oct 31 05:23:28 2017 +0000 ---------------------------------------------------------------------- be/src/runtime/coordinator.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d1594298/be/src/runtime/coordinator.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc index 5f6bd08..4a131b9 100644 --- a/be/src/runtime/coordinator.cc +++ b/be/src/runtime/coordinator.cc @@ -1124,12 +1124,12 @@ void Coordinator::UpdateFilter(const TUpdateFilterParams& params) { // Assign outgoing bloom filter. TBloomFilter& aggregated_filter = state->bloom_filter(); - filter_mem_tracker_->Release(aggregated_filter.directory.capacity()); + filter_mem_tracker_->Release(aggregated_filter.directory.size()); swap(rpc_params.bloom_filter, aggregated_filter); DCHECK(rpc_params.bloom_filter.always_false || rpc_params.bloom_filter.always_true || - rpc_params.bloom_filter.directory.size() != 0); + !rpc_params.bloom_filter.directory.empty()); + DCHECK(aggregated_filter.directory.empty()); rpc_params.__isset.bloom_filter = true; - DCHECK_EQ(aggregated_filter.directory.capacity(), 0); // Filter is complete, and can be released. state->Disable(filter_mem_tracker_); } @@ -1158,7 +1158,7 @@ void Coordinator::FilterState::ApplyUpdate(const TUpdateFilterParams& params, if (params.bloom_filter.always_true) { Disable(coord->filter_mem_tracker_); } else if (bloom_filter_.always_false) { - int64_t heap_space = params.bloom_filter.directory.capacity(); + int64_t heap_space = params.bloom_filter.directory.size(); if (!coord->filter_mem_tracker_->TryConsume(heap_space)) { VLOG_QUERY << "Not enough memory to allocate filter: " << PrettyPrinter::Print(heap_space, TUnit::BYTES) @@ -1186,9 +1186,9 @@ void Coordinator::FilterState::ApplyUpdate(const TUpdateFilterParams& params, void Coordinator::FilterState::Disable(MemTracker* tracker) { bloom_filter_.always_true = true; bloom_filter_.always_false = false; - int64_t capacity = bloom_filter_.directory.capacity(); + tracker->Release(bloom_filter_.directory.size()); bloom_filter_.directory.clear(); - tracker->Release(capacity); + bloom_filter_.directory.shrink_to_fit(); } const TUniqueId& Coordinator::query_id() const {
