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/08/22 05:52:15 UTC
[2/2] incubator-impala git commit: IMPALA-5158,
IMPALA-5236: account for unused buffer pool reservations
IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations
We were missing accounting for this, since it is part of the expected
difference between query and process memory consumption. The identity
that applies is is:
buffers allocated from system =
reservation + cached buffers - unused reservation
Where "cached buffers" includes free buffers and buffers attached to
clean pages. The reservation is accounted against queries and "buffers
allocated from system" is accounted against the process MemTracker.
Reporting this in a direct way required adding a MemTracker with a
negative consumption consumption, which fortunately did not require
any major changes to the MemTracker code.
Example output when applied to buffer pool branch:
Process: Limit=8.35 GB Total=579.18 MB Peak=590.41 MB
Buffer Pool: Free Buffers: Total=268.25 MB
Buffer Pool: Clean Pages: Total=172.25 MB
Buffer Pool: Unused Reservation: Total=-8.25 MB
Free Disk IO Buffers: Total=21.98 MB Peak=21.98 MB
RequestPool=default-pool: Total=12.07 MB Peak=71.58 MB
... <snip> ...
RequestPool=fe-eval-exprs: Total=0 Peak=4.00 KB
Untracked Memory: Total=112.88 MB
Testing:
Added a basic test for MemTrackers with negative metrics.
Change-Id: Idb1fa3110dc893321f9f4e8ced6b7ede12194dad
Reviewed-on: http://gerrit.cloudera.org:8080/7380
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/d0d0af27
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d0d0af27
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d0d0af27
Branch: refs/heads/master
Commit: d0d0af27863370d66a9f5c7fa902e08eac0a37d5
Parents: 1e1be6d
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Fri Jul 7 14:00:01 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Aug 22 04:37:14 2017 +0000
----------------------------------------------------------------------
be/src/runtime/exec-env.cc | 22 ++++++++++++-------
be/src/runtime/mem-tracker-test.cc | 23 ++++++++++++++++++-
be/src/runtime/mem-tracker.cc | 4 ++--
be/src/runtime/mem-tracker.h | 4 ++--
be/src/util/memory-metrics.cc | 39 +++++++++++++++++++++++----------
be/src/util/memory-metrics.h | 28 +++++++++++++----------
be/src/util/metrics-test.cc | 24 ++++++++++----------
be/src/util/metrics.h | 23 ++++++++++++++++++-
be/src/util/pretty-printer-test.cc | 6 +++++
be/src/util/pretty-printer.h | 6 ++---
common/thrift/metrics.json | 10 +++++++++
11 files changed, 136 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/runtime/exec-env.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 0b96e2a..02d07e8 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -282,14 +282,20 @@ Status ExecEnv::StartServices() {
// Limit of -1 means no memory limit.
mem_tracker_.reset(new MemTracker(AggregateMemoryMetrics::TOTAL_USED,
no_process_mem_limit ? -1 : bytes_limit, "Process"));
- if (buffer_pool_ != nullptr) {
- // Add BufferPool MemTrackers for cached memory that is not tracked against queries
- // but is included in process memory consumption.
- obj_pool_->Add(new MemTracker(BufferPoolMetric::FREE_BUFFER_BYTES, -1,
- "Buffer Pool: Free Buffers", mem_tracker_.get()));
- obj_pool_->Add(new MemTracker(BufferPoolMetric::CLEAN_PAGE_BYTES, -1,
- "Buffer Pool: Clean Pages", mem_tracker_.get()));
- }
+ // Add BufferPool MemTrackers for cached memory that is not tracked against queries
+ // but is included in process memory consumption.
+ obj_pool_->Add(new MemTracker(BufferPoolMetric::FREE_BUFFER_BYTES, -1,
+ "Buffer Pool: Free Buffers", mem_tracker_.get()));
+ obj_pool_->Add(new MemTracker(BufferPoolMetric::CLEAN_PAGE_BYTES, -1,
+ "Buffer Pool: Clean Pages", mem_tracker_.get()));
+ // Also need a MemTracker for unused reservations as a negative value. Unused
+ // reservations are counted against queries but not against the process memory
+ // consumption. This accounts for that difference.
+ IntGauge* negated_unused_reservation = obj_pool_->Add(new NegatedGauge<int64_t>(
+ MakeTMetricDef("negated_unused_reservation", TMetricKind::GAUGE, TUnit::BYTES),
+ BufferPoolMetric::UNUSED_RESERVATION_BYTES));
+ obj_pool_->Add(new MemTracker(negated_unused_reservation, -1,
+ "Buffer Pool: Unused Reservation", mem_tracker_.get()));
#ifndef ADDRESS_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.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/runtime/mem-tracker-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/mem-tracker-test.cc b/be/src/runtime/mem-tracker-test.cc
index 546b8ab..4aaac05 100644
--- a/be/src/runtime/mem-tracker-test.cc
+++ b/be/src/runtime/mem-tracker-test.cc
@@ -61,53 +61,74 @@ TEST(MemTestTest, ConsumptionMetric) {
md.__set_key("test");
md.__set_units(TUnit::BYTES);
md.__set_kind(TMetricKind::GAUGE);
- UIntGauge metric(md, 0);
+ IntGauge metric(md, 0);
EXPECT_EQ(metric.value(), 0);
+ TMetricDef neg_md;
+ neg_md.__set_key("neg_test");
+ neg_md.__set_units(TUnit::BYTES);
+ neg_md.__set_kind(TMetricKind::GAUGE);
+ NegatedGauge<int64_t> neg_metric(neg_md, &metric);
+
MemTracker t(&metric, 100, "");
+ MemTracker neg_t(&neg_metric, 100, "");
EXPECT_TRUE(t.has_limit());
EXPECT_EQ(t.consumption(), 0);
+ EXPECT_EQ(neg_t.consumption(), 0);
// Consume()/Release() arguments have no effect
t.Consume(150);
EXPECT_EQ(t.consumption(), 0);
EXPECT_EQ(t.peak_consumption(), 0);
EXPECT_FALSE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), 0);
t.Release(5);
EXPECT_EQ(t.consumption(), 0);
EXPECT_EQ(t.peak_consumption(), 0);
EXPECT_FALSE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), 0);
metric.Increment(10);
// consumption_ is only updated with consumption_metric_ after calls to
// Consume()/Release() with a non-zero value
t.Consume(1);
+ neg_t.Consume(1);
EXPECT_EQ(t.consumption(), 10);
EXPECT_EQ(t.peak_consumption(), 10);
+ EXPECT_EQ(neg_t.consumption(), -10);
metric.Increment(-5);
t.Consume(-1);
+ neg_t.Consume(1);
EXPECT_EQ(t.consumption(), 5);
EXPECT_EQ(t.peak_consumption(), 10);
EXPECT_FALSE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), -5);
metric.Increment(150);
t.Consume(1);
+ neg_t.Consume(1);
EXPECT_EQ(t.consumption(), 155);
EXPECT_EQ(t.peak_consumption(), 155);
EXPECT_TRUE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), -155);
metric.Increment(-150);
t.Consume(-1);
+ neg_t.Consume(1);
EXPECT_EQ(t.consumption(), 5);
EXPECT_EQ(t.peak_consumption(), 155);
EXPECT_FALSE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), -5);
// consumption_ is not updated when Consume()/Release() is called with a zero value
metric.Increment(10);
t.Consume(0);
+ neg_t.Consume(0);
EXPECT_EQ(t.consumption(), 5);
EXPECT_EQ(t.peak_consumption(), 155);
EXPECT_FALSE(t.LimitExceeded());
+ EXPECT_EQ(neg_t.consumption(), -5);
// Clean up.
metric.Increment(-15);
t.Consume(-1);
+ neg_t.Consume(-1);
}
TEST(MemTestTest, TrackerHierarchy) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/runtime/mem-tracker.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/mem-tracker.cc b/be/src/runtime/mem-tracker.cc
index 445f499..670a62e 100644
--- a/be/src/runtime/mem-tracker.cc
+++ b/be/src/runtime/mem-tracker.cc
@@ -75,8 +75,8 @@ MemTracker::MemTracker(RuntimeProfile* profile, int64_t byte_limit,
Init();
}
-MemTracker::MemTracker(UIntGauge* consumption_metric, int64_t byte_limit,
- const string& label, MemTracker* parent)
+MemTracker::MemTracker(IntGauge* consumption_metric,
+ int64_t byte_limit, const string& label, MemTracker* parent)
: limit_(byte_limit),
label_(label),
parent_(parent),
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/runtime/mem-tracker.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/mem-tracker.h b/be/src/runtime/mem-tracker.h
index 0d2f4d7..644353d 100644
--- a/be/src/runtime/mem-tracker.h
+++ b/be/src/runtime/mem-tracker.h
@@ -90,7 +90,7 @@ class MemTracker {
/// Consume()/Release() can still be called. This is used for the root process tracker
/// (if 'parent' is NULL). It is also to report on other categories of memory under the
/// process tracker, e.g. buffer pool free buffers (if 'parent - non-NULL).
- MemTracker(UIntGauge* consumption_metric, int64_t byte_limit = -1,
+ MemTracker(IntGauge* consumption_metric, int64_t byte_limit = -1,
const std::string& label = std::string(), MemTracker* parent = NULL);
~MemTracker();
@@ -400,7 +400,7 @@ class MemTracker {
/// If non-NULL, used to measure consumption (in bytes) rather than the values provided
/// to Consume()/Release(). Only used for the process tracker, thus parent_ should be
/// NULL if consumption_metric_ is set.
- UIntGauge* consumption_metric_;
+ IntGauge* consumption_metric_;
/// If non-NULL, counters from a corresponding ReservationTracker that should be
/// reported in logs and other diagnostics. Owned by this MemTracker. The counters
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/memory-metrics.cc
----------------------------------------------------------------------
diff --git a/be/src/util/memory-metrics.cc b/be/src/util/memory-metrics.cc
index 7d0d065..7c41bdf 100644
--- a/be/src/util/memory-metrics.cc
+++ b/be/src/util/memory-metrics.cc
@@ -32,11 +32,11 @@ using namespace strings;
DECLARE_bool(mmap_buffers);
-SumGauge<uint64_t>* AggregateMemoryMetrics::TOTAL_USED = nullptr;
-UIntGauge* AggregateMemoryMetrics::NUM_MAPS = nullptr;
-UIntGauge* AggregateMemoryMetrics::MAPPED_BYTES = nullptr;
-UIntGauge* AggregateMemoryMetrics::RSS = nullptr;
-UIntGauge* AggregateMemoryMetrics::ANON_HUGE_PAGE_BYTES = nullptr;
+SumGauge<int64_t>* AggregateMemoryMetrics::TOTAL_USED = nullptr;
+IntGauge* AggregateMemoryMetrics::NUM_MAPS = nullptr;
+IntGauge* AggregateMemoryMetrics::MAPPED_BYTES = nullptr;
+IntGauge* AggregateMemoryMetrics::RSS = nullptr;
+IntGauge* AggregateMemoryMetrics::ANON_HUGE_PAGE_BYTES = nullptr;
StringProperty* AggregateMemoryMetrics::THP_ENABLED = nullptr;
StringProperty* AggregateMemoryMetrics::THP_DEFRAG = nullptr;
StringProperty* AggregateMemoryMetrics::THP_KHUGEPAGED_DEFRAG = nullptr;
@@ -52,6 +52,7 @@ AsanMallocMetric* AsanMallocMetric::BYTES_ALLOCATED = nullptr;
BufferPoolMetric* BufferPoolMetric::LIMIT = nullptr;
BufferPoolMetric* BufferPoolMetric::SYSTEM_ALLOCATED = nullptr;
BufferPoolMetric* BufferPoolMetric::RESERVED = nullptr;
+BufferPoolMetric* BufferPoolMetric::UNUSED_RESERVATION_BYTES = nullptr;
BufferPoolMetric* BufferPoolMetric::NUM_FREE_BUFFERS = nullptr;
BufferPoolMetric* BufferPoolMetric::FREE_BUFFER_BYTES = nullptr;
BufferPoolMetric* BufferPoolMetric::CLEAN_PAGES_LIMIT = nullptr;
@@ -73,7 +74,7 @@ Status impala::RegisterMemoryMetrics(MetricGroup* metrics, bool register_jvm_met
// Add compound metrics that track totals across malloc and the buffer pool.
// total-used should track the total physical memory in use.
- vector<UIntGauge*> used_metrics;
+ vector<IntGauge*> used_metrics;
if (FLAGS_mmap_buffers && global_reservations != nullptr) {
// If we mmap() buffers, the buffers are not allocated via malloc. Ensure they are
// properly tracked.
@@ -109,19 +110,19 @@ Status impala::RegisterMemoryMetrics(MetricGroup* metrics, bool register_jvm_met
#endif
MetricGroup* aggregate_metrics = metrics->GetOrCreateChildGroup("memory");
AggregateMemoryMetrics::TOTAL_USED = aggregate_metrics->RegisterMetric(
- new SumGauge<uint64_t>(MetricDefs::Get("memory.total-used"), used_metrics));
+ new SumGauge<int64_t>(MetricDefs::Get("memory.total-used"), used_metrics));
if (register_jvm_metrics) {
RETURN_IF_ERROR(JvmMetric::InitMetrics(metrics->GetOrCreateChildGroup("jvm")));
}
if (MemInfo::HaveSmaps()) {
AggregateMemoryMetrics::NUM_MAPS =
- aggregate_metrics->AddGauge<uint64_t>("memory.num-maps", 0U);
+ aggregate_metrics->AddGauge<int64_t>("memory.num-maps", 0U);
AggregateMemoryMetrics::MAPPED_BYTES =
- aggregate_metrics->AddGauge<uint64_t>("memory.mapped-bytes", 0U);
- AggregateMemoryMetrics::RSS = aggregate_metrics->AddGauge<uint64_t>("memory.rss", 0U);
+ aggregate_metrics->AddGauge<int64_t>("memory.mapped-bytes", 0U);
+ AggregateMemoryMetrics::RSS = aggregate_metrics->AddGauge<int64_t>("memory.rss", 0U);
AggregateMemoryMetrics::ANON_HUGE_PAGE_BYTES =
- aggregate_metrics->AddGauge<uint64_t>("memory.anon-huge-page-bytes", 0U);
+ aggregate_metrics->AddGauge<int64_t>("memory.anon-huge-page-bytes", 0U);
}
ThpConfig thp_config = MemInfo::ParseThpConfig();
AggregateMemoryMetrics::THP_ENABLED =
@@ -233,6 +234,10 @@ Status BufferPoolMetric::InitMetrics(MetricGroup* metrics,
RESERVED = metrics->RegisterMetric(
new BufferPoolMetric(MetricDefs::Get("buffer-pool.reserved"),
BufferPoolMetricType::RESERVED, global_reservations, buffer_pool));
+ UNUSED_RESERVATION_BYTES = metrics->RegisterMetric(
+ new BufferPoolMetric(MetricDefs::Get("buffer-pool.unused-reservation-bytes"),
+ BufferPoolMetricType::UNUSED_RESERVATION_BYTES, global_reservations,
+ buffer_pool));
NUM_FREE_BUFFERS = metrics->RegisterMetric(
new BufferPoolMetric(MetricDefs::Get("buffer-pool.free-buffers"),
BufferPoolMetricType::NUM_FREE_BUFFERS, global_reservations, buffer_pool));
@@ -253,7 +258,7 @@ Status BufferPoolMetric::InitMetrics(MetricGroup* metrics,
BufferPoolMetric::BufferPoolMetric(const TMetricDef& def, BufferPoolMetricType type,
ReservationTracker* global_reservations, BufferPool* buffer_pool)
- : UIntGauge(def, 0),
+ : IntGauge(def, 0),
type_(type),
global_reservations_(global_reservations),
buffer_pool_(buffer_pool) {}
@@ -269,6 +274,16 @@ void BufferPoolMetric::CalculateValue() {
case BufferPoolMetricType::RESERVED:
value_ = global_reservations_->GetReservation();
break;
+ case BufferPoolMetricType::UNUSED_RESERVATION_BYTES: {
+ // Estimate the unused reservation based on other aggregate values, defined as
+ // the total bytes of reservation where there is no corresponding buffer in use
+ // by a client. Buffers are either in-use, free buffers, or attached to clean pages.
+ int64_t total_used_reservation = buffer_pool_->GetSystemBytesAllocated()
+ - buffer_pool_->GetFreeBufferBytes()
+ - buffer_pool_->GetCleanPageBytes();
+ value_ = global_reservations_->GetReservation() - total_used_reservation;
+ break;
+ }
case BufferPoolMetricType::NUM_FREE_BUFFERS:
value_ = buffer_pool_->GetNumFreeBuffers();
break;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/memory-metrics.h
----------------------------------------------------------------------
diff --git a/be/src/util/memory-metrics.h b/be/src/util/memory-metrics.h
index 4f65933..e1f1e4a 100644
--- a/be/src/util/memory-metrics.h
+++ b/be/src/util/memory-metrics.h
@@ -44,23 +44,23 @@ class AggregateMemoryMetrics {
/// including JVM memory), which is either in use by queries or cached by the BufferPool
/// or the malloc implementation.
/// TODO: IMPALA-691 - consider changing this to include JVM memory.
- static SumGauge<uint64_t>* TOTAL_USED;
+ static SumGauge<int64_t>* TOTAL_USED;
/// The total number of virtual memory regions for the process.
/// The value must be refreshed by calling Refresh().
- static UIntGauge* NUM_MAPS;
+ static IntGauge* NUM_MAPS;
/// The total size of virtual memory regions for the process.
/// The value must be refreshed by calling Refresh().
- static UIntGauge* MAPPED_BYTES;
+ static IntGauge* MAPPED_BYTES;
/// The total RSS of all virtual memory regions for the process.
/// The value must be refreshed by calling Refresh().
- static UIntGauge* RSS;
+ static IntGauge* RSS;
/// The total RSS of all virtual memory regions for the process.
/// The value must be refreshed by calling Refresh().
- static UIntGauge* ANON_HUGE_PAGE_BYTES;
+ static IntGauge* ANON_HUGE_PAGE_BYTES;
/// The string reporting the /enabled setting for transparent huge pages.
/// The value must be refreshed by calling Refresh().
@@ -79,7 +79,7 @@ class AggregateMemoryMetrics {
};
/// Specialised metric which exposes numeric properties from tcmalloc.
-class TcmallocMetric : public UIntGauge {
+class TcmallocMetric : public IntGauge {
public:
/// Number of bytes allocated by tcmalloc, currently used by the application.
static TcmallocMetric* BYTES_IN_USE;
@@ -102,9 +102,9 @@ class TcmallocMetric : public UIntGauge {
/// Derived metric computing the amount of physical memory (in bytes) used by the
/// process, including that actually in use and free bytes reserved by tcmalloc. Does not
/// include the tcmalloc metadata.
- class PhysicalBytesMetric : public UIntGauge {
+ class PhysicalBytesMetric : public IntGauge {
public:
- PhysicalBytesMetric(const TMetricDef& def) : UIntGauge(def, 0) { }
+ PhysicalBytesMetric(const TMetricDef& def) : IntGauge(def, 0) { }
private:
virtual void CalculateValue() {
@@ -122,7 +122,7 @@ class TcmallocMetric : public UIntGauge {
const std::string tcmalloc_var_;
TcmallocMetric(const TMetricDef& def, const std::string& tcmalloc_var)
- : UIntGauge(def, 0), tcmalloc_var_(tcmalloc_var) { }
+ : IntGauge(def, 0), tcmalloc_var_(tcmalloc_var) { }
virtual void CalculateValue() {
#ifndef ADDRESS_SANITIZER
@@ -135,9 +135,9 @@ class TcmallocMetric : public UIntGauge {
/// Alternative to TCMallocMetric if we're running under Address Sanitizer, which
/// does not provide the same metrics.
-class AsanMallocMetric : public UIntGauge {
+class AsanMallocMetric : public IntGauge {
public:
- AsanMallocMetric(const TMetricDef& def) : UIntGauge(def, 0) {}
+ AsanMallocMetric(const TMetricDef& def) : IntGauge(def, 0) {}
static AsanMallocMetric* BYTES_ALLOCATED;
private:
virtual void CalculateValue() override {
@@ -190,7 +190,7 @@ class JvmMetric : public IntGauge {
};
/// Metric that reports information about the buffer pool.
-class BufferPoolMetric : public UIntGauge {
+class BufferPoolMetric : public IntGauge {
public:
static Status InitMetrics(MetricGroup* metrics, ReservationTracker* global_reservations,
BufferPool* buffer_pool) WARN_UNUSED_RESULT;
@@ -199,6 +199,7 @@ class BufferPoolMetric : public UIntGauge {
static BufferPoolMetric* LIMIT;
static BufferPoolMetric* SYSTEM_ALLOCATED;
static BufferPoolMetric* RESERVED;
+ static BufferPoolMetric* UNUSED_RESERVATION_BYTES;
static BufferPoolMetric* NUM_FREE_BUFFERS;
static BufferPoolMetric* FREE_BUFFER_BYTES;
static BufferPoolMetric* CLEAN_PAGES_LIMIT;
@@ -217,6 +218,9 @@ class BufferPoolMetric : public UIntGauge {
// are fulfilled, or > SYSTEM_ALLOCATED because of additional memory cached by
// BufferPool. Always <= LIMIT.
RESERVED,
+ // Total bytes of reservations that have not been used to allocate buffers from the
+ // pool.
+ UNUSED_RESERVATION_BYTES,
NUM_FREE_BUFFERS, // Total number of free buffers in BufferPool.
FREE_BUFFER_BYTES, // Total bytes of free buffers in BufferPool.
CLEAN_PAGES_LIMIT, // Limit on number of clean pages in BufferPool.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/metrics-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/metrics-test.cc b/be/src/util/metrics-test.cc
index f25d5ad..08ca266 100644
--- a/be/src/util/metrics-test.cc
+++ b/be/src/util/metrics-test.cc
@@ -219,8 +219,8 @@ TEST_F(MetricsTest, MemMetric) {
MetricGroup metrics("MemMetrics");
ASSERT_OK(RegisterMemoryMetrics(&metrics, false, nullptr, nullptr));
// Smoke test to confirm that tcmalloc metrics are returning reasonable values.
- UIntGauge* bytes_in_use =
- metrics.FindMetricForTesting<UIntGauge>("tcmalloc.bytes-in-use");
+ IntGauge* bytes_in_use =
+ metrics.FindMetricForTesting<IntGauge>("tcmalloc.bytes-in-use");
ASSERT_TRUE(bytes_in_use != NULL);
uint64_t cur_in_use = bytes_in_use->value();
@@ -232,17 +232,17 @@ TEST_F(MetricsTest, MemMetric) {
scoped_ptr<vector<uint64_t>> chunk(new vector<uint64_t>(100 * 1024 * 1024));
EXPECT_GT(bytes_in_use->value(), cur_in_use);
- UIntGauge* total_bytes_reserved =
- metrics.FindMetricForTesting<UIntGauge>("tcmalloc.total-bytes-reserved");
+ IntGauge* total_bytes_reserved =
+ metrics.FindMetricForTesting<IntGauge>("tcmalloc.total-bytes-reserved");
ASSERT_TRUE(total_bytes_reserved != NULL);
ASSERT_GT(total_bytes_reserved->value(), 0);
- UIntGauge* pageheap_free_bytes =
- metrics.FindMetricForTesting<UIntGauge>("tcmalloc.pageheap-free-bytes");
+ IntGauge* pageheap_free_bytes =
+ metrics.FindMetricForTesting<IntGauge>("tcmalloc.pageheap-free-bytes");
ASSERT_TRUE(pageheap_free_bytes != NULL);
- UIntGauge* pageheap_unmapped_bytes =
- metrics.FindMetricForTesting<UIntGauge>("tcmalloc.pageheap-unmapped-bytes");
+ IntGauge* pageheap_unmapped_bytes =
+ metrics.FindMetricForTesting<IntGauge>("tcmalloc.pageheap-unmapped-bytes");
EXPECT_TRUE(pageheap_unmapped_bytes != NULL);
#endif
}
@@ -250,13 +250,13 @@ TEST_F(MetricsTest, MemMetric) {
TEST_F(MetricsTest, JvmMetrics) {
MetricGroup metrics("JvmMetrics");
ASSERT_OK(RegisterMemoryMetrics(&metrics, true, nullptr, nullptr));
- UIntGauge* jvm_total_used =
- metrics.GetOrCreateChildGroup("jvm")->FindMetricForTesting<UIntGauge>(
+ IntGauge* jvm_total_used =
+ metrics.GetOrCreateChildGroup("jvm")->FindMetricForTesting<IntGauge>(
"jvm.total.current-usage-bytes");
ASSERT_TRUE(jvm_total_used != NULL);
EXPECT_GT(jvm_total_used->value(), 0);
- UIntGauge* jvm_peak_total_used =
- metrics.GetOrCreateChildGroup("jvm")->FindMetricForTesting<UIntGauge>(
+ IntGauge* jvm_peak_total_used =
+ metrics.GetOrCreateChildGroup("jvm")->FindMetricForTesting<IntGauge>(
"jvm.total.peak-current-usage-bytes");
ASSERT_TRUE(jvm_peak_total_used != NULL);
EXPECT_GT(jvm_peak_total_used->value(), 0);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/metrics.h
----------------------------------------------------------------------
diff --git a/be/src/util/metrics.h b/be/src/util/metrics.h
index 1296e06..12d6df3 100644
--- a/be/src/util/metrics.h
+++ b/be/src/util/metrics.h
@@ -232,6 +232,24 @@ class SumGauge : public SimpleMetric<T, TMetricKind::GAUGE> {
std::vector<SimpleMetric<T, TMetricKind::GAUGE>*> metrics_;
};
+// Gauge metric that negates another gauge.
+template <typename T>
+class NegatedGauge : public SimpleMetric<T, TMetricKind::GAUGE> {
+ public:
+ NegatedGauge(const TMetricDef& metric_def,
+ SimpleMetric<T, TMetricKind::GAUGE>* metric)
+ : SimpleMetric<T, TMetricKind::GAUGE>(metric_def, 0), metric_(metric) {}
+ virtual ~NegatedGauge() {}
+
+ private:
+ virtual void CalculateValue() override {
+ this->value_ = -metric_->value();
+ }
+
+ /// The metric to be negated.
+ SimpleMetric<T, TMetricKind::GAUGE>* metric_;
+};
+
/// Container for a set of metrics. A MetricGroup owns the memory for every metric
/// contained within it (see Add*() to create commonly used metric
/// types). Metrics are 'registered' with a MetricGroup, once registered they cannot be
@@ -364,13 +382,16 @@ class MetricGroup {
/// We write 'Int' as a placeholder for all integer types.
typedef class SimpleMetric<int64_t, TMetricKind::GAUGE> IntGauge;
-typedef class SimpleMetric<uint64_t, TMetricKind::GAUGE> UIntGauge;
typedef class SimpleMetric<double, TMetricKind::GAUGE> DoubleGauge;
typedef class SimpleMetric<int64_t, TMetricKind::COUNTER> IntCounter;
typedef class SimpleMetric<bool, TMetricKind::PROPERTY> BooleanProperty;
typedef class SimpleMetric<std::string, TMetricKind::PROPERTY> StringProperty;
+/// Convenience method to instantiate a TMetricDef with a subset of its fields defined.
+/// Most externally-visible metrics should be defined in metrics.json and retrieved via
+/// MetricDefs::Get(). This alternative method of instantiating TMetricDefs is only used
+/// in special cases where the regular approach is unsuitable.
TMetricDef MakeTMetricDef(const std::string& key, TMetricKind::type kind,
TUnit::type unit);
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/pretty-printer-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/pretty-printer-test.cc b/be/src/util/pretty-printer-test.cc
index fbf9827..6daa131 100644
--- a/be/src/util/pretty-printer-test.cc
+++ b/be/src/util/pretty-printer-test.cc
@@ -63,6 +63,12 @@ TEST(PrettyPrinterTest, Bytes) {
EXPECT_EQ(PrettyPrinter::Print(1024 * 1024, TUnit::BYTES), "1.00 MB");
EXPECT_EQ(PrettyPrinter::Print(1024 * 1024 * 1024, TUnit::BYTES), "1.00 GB");
+ // Negative values
+ EXPECT_EQ(PrettyPrinter::Print(-2, TUnit::BYTES), "-2.00 B");
+ EXPECT_EQ(PrettyPrinter::Print(-1024, TUnit::BYTES), "-1.00 KB");
+ EXPECT_EQ(PrettyPrinter::Print(-1024 * 1024, TUnit::BYTES), "-1.00 MB");
+ EXPECT_EQ(PrettyPrinter::Print(-1024 * 1024 * 1024, TUnit::BYTES), "-1.00 GB");
+
EXPECT_EQ(PrettyPrinter::Print(1536, TUnit::BYTES), "1.50 KB");
EXPECT_EQ(PrettyPrinter::Print(1536.0, TUnit::BYTES), "1.50 KB");
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/be/src/util/pretty-printer.h
----------------------------------------------------------------------
diff --git a/be/src/util/pretty-printer.h b/be/src/util/pretty-printer.h
index 6a3fa9a..5e76db1 100644
--- a/be/src/util/pretty-printer.h
+++ b/be/src/util/pretty-printer.h
@@ -204,13 +204,13 @@ class PrettyPrinter {
if (value == 0) {
*unit = "";
return value;
- } else if (value >= GIGABYTE) {
+ } else if (value >= GIGABYTE || value <= -GIGABYTE) {
*unit = "GB";
return value / (double) GIGABYTE;
- } else if (value >= MEGABYTE ) {
+ } else if (value >= MEGABYTE || value <= -MEGABYTE ) {
*unit = "MB";
return value / (double) MEGABYTE;
- } else if (value >= KILOBYTE) {
+ } else if (value >= KILOBYTE || value <= -KILOBYTE) {
*unit = "KB";
return value / (double) KILOBYTE;
} else {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d0d0af27/common/thrift/metrics.json
----------------------------------------------------------------------
diff --git a/common/thrift/metrics.json b/common/thrift/metrics.json
index 6c4f07d..567e1bf 100644
--- a/common/thrift/metrics.json
+++ b/common/thrift/metrics.json
@@ -1156,6 +1156,16 @@
"key": "buffer-pool.reserved"
},
{
+ "description": "Total bytes of buffer reservations by Impala subsystems that are currently unused",
+ "contexts": [
+ "IMPALAD"
+ ],
+ "label": "Buffer Pool Unused Reservation Bytes.",
+ "units": "BYTES",
+ "kind": "GAUGE",
+ "key": "buffer-pool.unused-reservation-bytes"
+ },
+ {
"description": "Total number of free buffers cached in the buffer pool.",
"contexts": [
"IMPALAD"