You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bo...@apache.org on 2019/02/27 10:28:21 UTC
[impala] 04/04: IMPALA-8223: remove {mem-pool,
hash-table}.total-bytes
This is an automated email from the ASF dual-hosted git repository.
boroknagyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit adced30896ade0f71e64cc2d5bad1cd9f43119b7
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Tue Feb 19 14:55:40 2019 -0800
IMPALA-8223: remove {mem-pool,hash-table}.total-bytes
These metrics have some issues (see the JIRA) and are not very relevant
in a world where most of the memory comes from the BufferPool
Change-Id: I61c8ca1ddf20676252eae958bc2d293e0ec67ecf
Reviewed-on: http://gerrit.cloudera.org:8080/12528
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/hash-table.cc | 4 ----
be/src/exec/hdfs-orc-scanner.cc | 3 ---
be/src/runtime/mem-pool.cc | 9 ---------
be/src/util/impalad-metrics.cc | 12 ------------
be/src/util/impalad-metrics.h | 8 --------
common/thrift/metrics.json | 20 --------------------
tests/verifiers/metric_verifier.py | 2 --
7 files changed, 58 deletions(-)
diff --git a/be/src/exec/hash-table.cc b/be/src/exec/hash-table.cc
index 576deca..58a9212 100644
--- a/be/src/exec/hash-table.cc
+++ b/be/src/exec/hash-table.cc
@@ -425,9 +425,6 @@ void HashTable::Close() {
if ((num_buckets_ > LARGE_HT) || (num_probes_ > HEAVILY_USED)) VLOG(2) << PrintStats();
for (auto& data_page : data_pages_) allocator_->Free(move(data_page));
data_pages_.clear();
- if (ImpaladMetrics::HASH_TABLE_TOTAL_BYTES != NULL) {
- ImpaladMetrics::HASH_TABLE_TOTAL_BYTES->Increment(-total_data_page_size_);
- }
if (bucket_allocation_ != nullptr) allocator_->Free(move(bucket_allocation_));
}
@@ -505,7 +502,6 @@ bool HashTable::GrowNodeArray(Status* status) {
if (!status->ok() || allocation == nullptr) return false;
next_node_ = reinterpret_cast<DuplicateNode*>(allocation->data());
data_pages_.push_back(move(allocation));
- ImpaladMetrics::HASH_TABLE_TOTAL_BYTES->Increment(DATA_PAGE_SIZE);
node_remaining_current_page_ = DATA_PAGE_SIZE / sizeof(DuplicateNode);
total_data_page_size_ += DATA_PAGE_SIZE;
return true;
diff --git a/be/src/exec/hdfs-orc-scanner.cc b/be/src/exec/hdfs-orc-scanner.cc
index a0083bf..a9ef594 100644
--- a/be/src/exec/hdfs-orc-scanner.cc
+++ b/be/src/exec/hdfs-orc-scanner.cc
@@ -67,7 +67,6 @@ void HdfsOrcScanner::OrcMemPool::FreeAll() {
}
mem_tracker_->Release(total_bytes_released);
chunk_sizes_.clear();
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(-total_bytes_released);
}
// orc-reader will not check the malloc result. We throw an exception if we can't
@@ -83,7 +82,6 @@ char* HdfsOrcScanner::OrcMemPool::malloc(uint64_t size) {
throw ResourceError(Status(TErrorCode::MEM_ALLOC_FAILED, size));
}
chunk_sizes_[addr] = size;
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(size);
return addr;
}
@@ -93,7 +91,6 @@ void HdfsOrcScanner::OrcMemPool::free(char* p) {
std::free(p);
int64_t size = chunk_sizes_[p];
mem_tracker_->Release(size);
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(-size);
chunk_sizes_.erase(p);
}
diff --git a/be/src/runtime/mem-pool.cc b/be/src/runtime/mem-pool.cc
index c0aa438..da8e865 100644
--- a/be/src/runtime/mem-pool.cc
+++ b/be/src/runtime/mem-pool.cc
@@ -52,9 +52,6 @@ MemPool::ChunkInfo::ChunkInfo(int64_t size, uint8_t* buf)
: data(buf),
size(size),
allocated_bytes(0) {
- if (ImpaladMetrics::MEM_POOL_TOTAL_BYTES != NULL) {
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(size);
- }
}
MemPool::~MemPool() {
@@ -65,9 +62,6 @@ MemPool::~MemPool() {
}
DCHECK(chunks_.empty()) << "Must call FreeAll() or AcquireData() for this pool";
- if (ImpaladMetrics::MEM_POOL_TOTAL_BYTES != NULL) {
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(-total_bytes_released);
- }
DCHECK_EQ(zero_length_region_, MEM_POOL_POISON);
}
@@ -96,9 +90,6 @@ void MemPool::FreeAll() {
total_reserved_bytes_ = 0;
mem_tracker_->Release(total_bytes_released);
- if (ImpaladMetrics::MEM_POOL_TOTAL_BYTES != NULL) {
- ImpaladMetrics::MEM_POOL_TOTAL_BYTES->Increment(-total_bytes_released);
- }
}
bool MemPool::FindChunk(int64_t min_size, bool check_limits) noexcept {
diff --git a/be/src/util/impalad-metrics.cc b/be/src/util/impalad-metrics.cc
index e4386ca..6db82e0 100644
--- a/be/src/util/impalad-metrics.cc
+++ b/be/src/util/impalad-metrics.cc
@@ -46,10 +46,6 @@ const char* ImpaladMetricKeys::TOTAL_SCAN_RANGES_PROCESSED =
"impala-server.scan-ranges.total";
const char* ImpaladMetricKeys::NUM_SCAN_RANGES_MISSING_VOLUME_ID =
"impala-server.scan-ranges.num-missing-volume-id";
-const char* ImpaladMetricKeys::MEM_POOL_TOTAL_BYTES =
- "impala-server.mem-pool.total-bytes";
-const char* ImpaladMetricKeys::HASH_TABLE_TOTAL_BYTES =
- "impala-server.hash-table.total-bytes";
const char* ImpaladMetricKeys::IO_MGR_NUM_OPEN_FILES =
"impala-server.io-mgr.num-open-files";
const char* ImpaladMetricKeys::IO_MGR_BYTES_READ =
@@ -131,7 +127,6 @@ const char* ImpaladMetricKeys::HEDGED_READ_OPS_WIN =
// These are created by impala-server during startup.
// =======
// Counters
-IntGauge* ImpaladMetrics::HASH_TABLE_TOTAL_BYTES = NULL;
IntCounter* ImpaladMetrics::BACKEND_NUM_QUERIES_EXECUTED = NULL;
IntGauge* ImpaladMetrics::BACKEND_NUM_QUERIES_EXECUTING = NULL;
IntCounter* ImpaladMetrics::IMPALA_SERVER_NUM_QUERIES = NULL;
@@ -174,7 +169,6 @@ IntGauge* ImpaladMetrics::IO_MGR_NUM_FILE_HANDLES_OUTSTANDING = NULL;
IntGauge* ImpaladMetrics::IO_MGR_CACHED_FILE_HANDLES_HIT_COUNT = NULL;
IntGauge* ImpaladMetrics::IO_MGR_CACHED_FILE_HANDLES_MISS_COUNT = NULL;
IntGauge* ImpaladMetrics::IO_MGR_TOTAL_BYTES = NULL;
-IntGauge* ImpaladMetrics::MEM_POOL_TOTAL_BYTES = NULL;
IntGauge* ImpaladMetrics::NUM_FILES_OPEN_FOR_INSERT = NULL;
IntGauge* ImpaladMetrics::NUM_QUERIES_REGISTERED = NULL;
IntGauge* ImpaladMetrics::RESULTSET_CACHE_TOTAL_NUM_ROWS = NULL;
@@ -280,12 +274,6 @@ void ImpaladMetrics::CreateMetrics(MetricGroup* m) {
NUM_RANGES_MISSING_VOLUME_ID = m->AddCounter(
ImpaladMetricKeys::NUM_SCAN_RANGES_MISSING_VOLUME_ID, 0);
- // Initialize memory usage metrics
- MEM_POOL_TOTAL_BYTES = m->AddGauge(
- ImpaladMetricKeys::MEM_POOL_TOTAL_BYTES, 0);
- HASH_TABLE_TOTAL_BYTES = m->AddGauge(
- ImpaladMetricKeys::HASH_TABLE_TOTAL_BYTES, 0);
-
// Initialize insert metrics
NUM_FILES_OPEN_FOR_INSERT = m->AddGauge(
ImpaladMetricKeys::NUM_FILES_OPEN_FOR_INSERT, 0);
diff --git a/be/src/util/impalad-metrics.h b/be/src/util/impalad-metrics.h
index 05babaa..2a25e86 100644
--- a/be/src/util/impalad-metrics.h
+++ b/be/src/util/impalad-metrics.h
@@ -64,12 +64,6 @@ class ImpaladMetricKeys {
/// Number of scan ranges with missing volume id metadata
static const char* NUM_SCAN_RANGES_MISSING_VOLUME_ID;
- /// Number of bytes currently in use across all mem pools
- static const char* MEM_POOL_TOTAL_BYTES;
-
- /// Number of bytes currently in use across all hash tables
- static const char* HASH_TABLE_TOTAL_BYTES;
-
/// Number of files currently opened by the io mgr
static const char* IO_MGR_NUM_OPEN_FILES;
@@ -210,7 +204,6 @@ class ImpaladMetricKeys {
class ImpaladMetrics {
public:
// Counters
- static IntGauge* HASH_TABLE_TOTAL_BYTES;
static IntCounter* BACKEND_NUM_QUERIES_EXECUTED;
/// BACKEND_NUM_QUERIES_EXECUTING is used to determine when the backend has quiesced
/// and can be safely shut down without causing query failures. See IMPALA-7931 for
@@ -261,7 +254,6 @@ class ImpaladMetrics {
static IntGauge* IO_MGR_CACHED_FILE_HANDLES_HIT_COUNT;
static IntGauge* IO_MGR_CACHED_FILE_HANDLES_MISS_COUNT;
static IntGauge* IO_MGR_TOTAL_BYTES;
- static IntGauge* MEM_POOL_TOTAL_BYTES;
static IntGauge* NUM_FILES_OPEN_FOR_INSERT;
static IntGauge* NUM_QUERIES_REGISTERED;
static IntGauge* RESULTSET_CACHE_TOTAL_NUM_ROWS;
diff --git a/common/thrift/metrics.json b/common/thrift/metrics.json
index ed34259..b18bc1f 100644
--- a/common/thrift/metrics.json
+++ b/common/thrift/metrics.json
@@ -360,16 +360,6 @@
"key": "impala-server.backends.client-cache.total-clients"
},
{
- "description": "The current size of all allocated hash tables.",
- "contexts": [
- "IMPALAD"
- ],
- "label": "Hash Tables Size",
- "units": "BYTES",
- "kind": "GAUGE",
- "key": "impala-server.hash-table.total-bytes"
- },
- {
"description": "The total number of bytes read by the IO manager.",
"contexts": [
"IMPALAD"
@@ -460,16 +450,6 @@
"key": "impala-server.io-mgr.total-bytes"
},
{
- "description": "The current size of the memory pool shared by all queries",
- "contexts": [
- "IMPALAD"
- ],
- "label": "Memory Pool Size",
- "units": "BYTES",
- "kind": "GAUGE",
- "key": "impala-server.mem-pool.total-bytes"
- },
- {
"description": "The number of HDFS files currently open for writing.",
"contexts": [
"IMPALAD"
diff --git a/tests/verifiers/metric_verifier.py b/tests/verifiers/metric_verifier.py
index 0c1777b..747c7ee 100644
--- a/tests/verifiers/metric_verifier.py
+++ b/tests/verifiers/metric_verifier.py
@@ -24,9 +24,7 @@ METRIC_LIST = [
# "impala-server.backends.client-cache.clients-in-use", disabled as a
# work-around due to IMPALA-3327.
#"impala-server.backends.client-cache.clients-in-use",
- "impala-server.hash-table.total-bytes",
"impala-server.io-mgr.num-open-files",
- "impala-server.mem-pool.total-bytes",
"impala-server.num-files-open-for-insert",
"impala-server.scan-ranges.num-missing-volume-id",
# Buffer pool pages belong to specific queries. Therefore there should be