This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 6889b14 [util] separate generalized CacheMetrics
6889b14 is described below
commit 6889b14be66c2e276a3878da5e88b01e73a9c45d
Author: Alexey Serbin <[email protected]>
AuthorDate: Mon Mar 11 00:30:58 2019 -0700
[util] separate generalized CacheMetrics
Separated the generic counters into CacheMetrics to make it possible
to use CacheMetrics for other cache types.
This changelist also addresses KUDU-2749, separating BlockCacheMetrics
and FileCacheMetrics.
These changes are introduced to accommodate a few follow-up
modifications introducing a TTL-based cache.
Change-Id: Ic77b937e23c600763ec987a96ce98a47ad95ee18
Reviewed-on: http://gerrit.cloudera.org:8080/12776
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <[email protected]>
---
src/kudu/cfile/block_cache.cc | 6 +-
src/kudu/util/CMakeLists.txt | 3 +-
.../{cache_metrics.cc => block_cache_metrics.cc} | 24 ++++----
.../{cache_metrics.h => block_cache_metrics.h} | 21 ++-----
src/kudu/util/cache-test.cc | 11 +++-
src/kudu/util/cache.cc | 26 ++++----
src/kudu/util/cache.h | 7 +--
src/kudu/util/cache_metrics.h | 11 ++--
src/kudu/util/file_cache.cc | 6 +-
src/kudu/util/file_cache_metrics.cc | 70 ++++++++++++++++++++++
.../util/{cache_metrics.h => file_cache_metrics.h} | 21 ++-----
src/kudu/util/nvm_cache.cc | 16 ++---
12 files changed, 141 insertions(+), 81 deletions(-)
diff --git a/src/kudu/cfile/block_cache.cc b/src/kudu/cfile/block_cache.cc
index ff7219d..1df5c19 100644
--- a/src/kudu/cfile/block_cache.cc
+++ b/src/kudu/cfile/block_cache.cc
@@ -19,6 +19,7 @@
#include <cstddef>
#include <cstdint>
+#include <memory>
#include <ostream>
#include <string>
@@ -28,7 +29,9 @@
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/strings/substitute.h"
+#include "kudu/util/block_cache_metrics.h"
#include "kudu/util/cache.h"
+#include "kudu/util/cache_metrics.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/flag_validators.h"
#include "kudu/util/process_memory.h"
@@ -147,7 +150,8 @@ void BlockCache::Insert(BlockCache::PendingEntry* entry,
BlockCacheHandle* inser
}
void BlockCache::StartInstrumentation(const scoped_refptr<MetricEntity>&
metric_entity) {
- cache_->SetMetrics(metric_entity);
+ std::unique_ptr<BlockCacheMetrics> metrics(new
BlockCacheMetrics(metric_entity));
+ cache_->SetMetrics(std::move(metrics));
}
} // namespace cfile
diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt
index 8779ac1..d08e0ee 100644
--- a/src/kudu/util/CMakeLists.txt
+++ b/src/kudu/util/CMakeLists.txt
@@ -147,10 +147,10 @@ set(UTIL_SRCS
async_logger.cc
atomic.cc
bitmap.cc
+ block_cache_metrics.cc
bloom_filter.cc
bitmap.cc
cache.cc
- cache_metrics.cc
coding.cc
condition_variable.cc
cow_object.cc
@@ -167,6 +167,7 @@ set(UTIL_SRCS
faststring.cc
fault_injection.cc
file_cache.cc
+ file_cache_metrics.cc
flags.cc
flag_tags.cc
flag_validators.cc
diff --git a/src/kudu/util/cache_metrics.cc
b/src/kudu/util/block_cache_metrics.cc
similarity index 81%
rename from src/kudu/util/cache_metrics.cc
rename to src/kudu/util/block_cache_metrics.cc
index ac2fadf..60a946f 100644
--- a/src/kudu/util/cache_metrics.cc
+++ b/src/kudu/util/block_cache_metrics.cc
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-#include "kudu/util/cache_metrics.h"
+#include "kudu/util/block_cache_metrics.h"
#include "kudu/util/metrics.h"
@@ -51,17 +51,17 @@ METRIC_DEFINE_gauge_uint64(server, block_cache_usage,
"Block Cache Memory Usage"
namespace kudu {
-#define MINIT(member, x) member(METRIC_##x.Instantiate(entity))
-#define GINIT(member, x) member(METRIC_##x.Instantiate(entity, 0))
-CacheMetrics::CacheMetrics(const scoped_refptr<MetricEntity>& entity)
- : MINIT(inserts, block_cache_inserts),
- MINIT(lookups, block_cache_lookups),
- MINIT(evictions, block_cache_evictions),
- MINIT(cache_hits, block_cache_hits),
- MINIT(cache_hits_caching, block_cache_hits_caching),
- MINIT(cache_misses, block_cache_misses),
- MINIT(cache_misses_caching, block_cache_misses_caching),
- GINIT(cache_usage, block_cache_usage) {
+#define MINIT(member, x) member = METRIC_##x.Instantiate(entity)
+#define GINIT(member, x) member = METRIC_##x.Instantiate(entity, 0)
+BlockCacheMetrics::BlockCacheMetrics(const scoped_refptr<MetricEntity>&
entity) {
+ MINIT(inserts, block_cache_inserts);
+ MINIT(lookups, block_cache_lookups);
+ MINIT(evictions, block_cache_evictions);
+ MINIT(cache_hits, block_cache_hits);
+ MINIT(cache_hits_caching, block_cache_hits_caching);
+ MINIT(cache_misses, block_cache_misses);
+ MINIT(cache_misses_caching, block_cache_misses_caching);
+ GINIT(cache_usage, block_cache_usage);
}
#undef MINIT
#undef GINIT
diff --git a/src/kudu/util/cache_metrics.h b/src/kudu/util/block_cache_metrics.h
similarity index 60%
copy from src/kudu/util/cache_metrics.h
copy to src/kudu/util/block_cache_metrics.h
index 04a546b..9994ad6 100644
--- a/src/kudu/util/cache_metrics.h
+++ b/src/kudu/util/block_cache_metrics.h
@@ -14,29 +14,18 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_UTIL_CACHE_METRICS_H
-#define KUDU_UTIL_CACHE_METRICS_H
-#include <cstdint>
+#pragma once
#include "kudu/gutil/ref_counted.h"
-#include "kudu/util/metrics.h"
+#include "kudu/util/cache_metrics.h"
namespace kudu {
-struct CacheMetrics {
- explicit CacheMetrics(const scoped_refptr<MetricEntity>& metric_entity);
+class MetricEntity;
- scoped_refptr<Counter> inserts;
- scoped_refptr<Counter> lookups;
- scoped_refptr<Counter> evictions;
- scoped_refptr<Counter> cache_hits;
- scoped_refptr<Counter> cache_hits_caching;
- scoped_refptr<Counter> cache_misses;
- scoped_refptr<Counter> cache_misses_caching;
-
- scoped_refptr<AtomicGauge<uint64_t> > cache_usage;
+struct BlockCacheMetrics : public CacheMetrics {
+ explicit BlockCacheMetrics(const scoped_refptr<MetricEntity>& entity);
};
} // namespace kudu
-#endif /* KUDU_UTIL_CACHE_METRICS_H */
diff --git a/src/kudu/util/cache-test.cc b/src/kudu/util/cache-test.cc
index 3fd1d5f..38d4f03 100644
--- a/src/kudu/util/cache-test.cc
+++ b/src/kudu/util/cache-test.cc
@@ -2,21 +2,25 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "kudu/util/cache.h"
+
#include <cassert>
#include <cstring>
#include <memory>
#include <string>
+#include <utility>
#include <vector>
-#include <glog/logging.h>
#include <gflags/gflags.h>
#include <gflags/gflags_declare.h>
+#include <glog/logging.h>
#include <gtest/gtest.h>
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
-#include "kudu/util/cache.h"
+#include "kudu/util/block_cache_metrics.h"
+#include "kudu/util/cache_metrics.h"
#include "kudu/util/coding.h"
#include "kudu/util/env.h"
#include "kudu/util/faststring.h"
@@ -87,7 +91,8 @@ class CacheTest : public KuduTest,
scoped_refptr<MetricEntity> entity = METRIC_ENTITY_server.Instantiate(
&metric_registry_, "test");
- cache_->SetMetrics(entity);
+ std::unique_ptr<BlockCacheMetrics> metrics(new BlockCacheMetrics(entity));
+ cache_->SetMetrics(std::move(metrics));
}
int Lookup(int key) {
diff --git a/src/kudu/util/cache.cc b/src/kudu/util/cache.cc
index 00f2e52..5c10702 100644
--- a/src/kudu/util/cache.cc
+++ b/src/kudu/util/cache.cc
@@ -11,6 +11,7 @@
#include <mutex>
#include <ostream>
#include <string>
+#include <utility>
#include <vector>
#include <gflags/gflags.h>
@@ -52,6 +53,7 @@ TAG_FLAG(cache_memtracker_approximation_ratio, hidden);
using std::atomic;
using std::shared_ptr;
using std::string;
+using std::unique_ptr;
using std::vector;
namespace kudu {
@@ -445,7 +447,7 @@ int DetermineShardBits() {
class ShardedLRUCache : public Cache {
private:
shared_ptr<MemTracker> mem_tracker_;
- gscoped_ptr<CacheMetrics> metrics_;
+ unique_ptr<CacheMetrics> metrics_;
vector<LRUCache*> shards_;
// Number of bits of hash used to determine the shard.
@@ -488,27 +490,27 @@ class ShardedLRUCache : public Cache {
STLDeleteElements(&shards_);
}
- virtual Handle* Insert(PendingHandle* handle,
- Cache::EvictionCallback* eviction_callback) OVERRIDE {
+ Handle* Insert(PendingHandle* handle,
+ Cache::EvictionCallback* eviction_callback) override {
LRUHandle* h = reinterpret_cast<LRUHandle*>(DCHECK_NOTNULL(handle));
return shards_[Shard(h->hash)]->Insert(h, eviction_callback);
}
- virtual Handle* Lookup(const Slice& key, CacheBehavior caching) OVERRIDE {
+ Handle* Lookup(const Slice& key, CacheBehavior caching) override {
const uint32_t hash = HashSlice(key);
return shards_[Shard(hash)]->Lookup(key, hash, caching == EXPECT_IN_CACHE);
}
- virtual void Release(Handle* handle) OVERRIDE {
+ void Release(Handle* handle) override {
LRUHandle* h = reinterpret_cast<LRUHandle*>(handle);
shards_[Shard(h->hash)]->Release(handle);
}
- virtual void Erase(const Slice& key) OVERRIDE {
+ void Erase(const Slice& key) override {
const uint32_t hash = HashSlice(key);
shards_[Shard(hash)]->Erase(key, hash);
}
- virtual Slice Value(Handle* handle) OVERRIDE {
+ Slice Value(Handle* handle) override {
return reinterpret_cast<LRUHandle*>(handle)->value();
}
- virtual void SetMetrics(const scoped_refptr<MetricEntity>& entity) OVERRIDE {
+ void SetMetrics(std::unique_ptr<CacheMetrics> metrics) override {
// TODO(KUDU-2165): reuse of the Cache singleton across multiple
MiniCluster servers
// causes TSAN errors. So, we'll ensure that metrics only get attached
once, from
// whichever server starts first. This has the downside that, in test
builds, we won't
@@ -518,13 +520,13 @@ class ShardedLRUCache : public Cache {
CHECK(IsGTest()) << "Metrics should only be set once per Cache
singleton";
return;
}
- metrics_.reset(new CacheMetrics(entity));
+ metrics_ = std::move(metrics);
for (LRUCache* cache : shards_) {
cache->SetMetrics(metrics_.get());
}
}
- virtual PendingHandle* Allocate(Slice key, int val_len, int charge) OVERRIDE
{
+ PendingHandle* Allocate(Slice key, int val_len, int charge) override {
int key_len = key.size();
DCHECK_GE(key_len, 0);
DCHECK_GE(val_len, 0);
@@ -543,12 +545,12 @@ class ShardedLRUCache : public Cache {
return reinterpret_cast<PendingHandle*>(handle);
}
- virtual void Free(PendingHandle* h) OVERRIDE {
+ void Free(PendingHandle* h) override {
uint8_t* data = reinterpret_cast<uint8_t*>(h);
delete [] data;
}
- virtual uint8_t* MutableValue(PendingHandle* h) OVERRIDE {
+ uint8_t* MutableValue(PendingHandle* h) override {
return reinterpret_cast<LRUHandle*>(h)->mutable_val_ptr();
}
diff --git a/src/kudu/util/cache.h b/src/kudu/util/cache.h
index 82ef8c9..4c0c7eb 100644
--- a/src/kudu/util/cache.h
+++ b/src/kudu/util/cache.h
@@ -24,13 +24,12 @@
#include <string>
#include "kudu/gutil/macros.h"
-#include "kudu/gutil/ref_counted.h"
#include "kudu/util/slice.h"
namespace kudu {
class Cache;
-class MetricEntity;
+struct CacheMetrics;
enum CacheType {
DRAM_CACHE,
@@ -129,8 +128,8 @@ class Cache {
// to it have been released.
virtual void Erase(const Slice& key) = 0;
- // Pass a metric entity in order to start recoding metrics.
- virtual void SetMetrics(const scoped_refptr<MetricEntity>& metric_entity) =
0;
+ // Set the cache metrics to update corresponding counters accordingly.
+ virtual void SetMetrics(std::unique_ptr<CacheMetrics> metrics) = 0;
// ------------------------------------------------------------
// Insertion path
diff --git a/src/kudu/util/cache_metrics.h b/src/kudu/util/cache_metrics.h
index 04a546b..f77b227 100644
--- a/src/kudu/util/cache_metrics.h
+++ b/src/kudu/util/cache_metrics.h
@@ -14,8 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_UTIL_CACHE_METRICS_H
-#define KUDU_UTIL_CACHE_METRICS_H
+
+#pragma once
#include <cstdint>
@@ -25,8 +25,8 @@
namespace kudu {
struct CacheMetrics {
- explicit CacheMetrics(const scoped_refptr<MetricEntity>& metric_entity);
-
+ virtual ~CacheMetrics() {
+ }
scoped_refptr<Counter> inserts;
scoped_refptr<Counter> lookups;
scoped_refptr<Counter> evictions;
@@ -35,8 +35,7 @@ struct CacheMetrics {
scoped_refptr<Counter> cache_misses;
scoped_refptr<Counter> cache_misses_caching;
- scoped_refptr<AtomicGauge<uint64_t> > cache_usage;
+ scoped_refptr<AtomicGauge<uint64_t>> cache_usage;
};
} // namespace kudu
-#endif /* KUDU_UTIL_CACHE_METRICS_H */
diff --git a/src/kudu/util/file_cache.cc b/src/kudu/util/file_cache.cc
index 3f987d7..d1bb92f 100644
--- a/src/kudu/util/file_cache.cc
+++ b/src/kudu/util/file_cache.cc
@@ -36,11 +36,12 @@
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/array_view.h"
#include "kudu/util/cache.h"
+#include "kudu/util/cache_metrics.h"
#include "kudu/util/countdown_latch.h"
#include "kudu/util/env.h"
+#include "kudu/util/file_cache_metrics.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/locks.h"
-#include "kudu/util/metrics.h" // IWYU pragma: keep
#include "kudu/util/monotime.h"
#include "kudu/util/once.h"
#include "kudu/util/slice.h"
@@ -461,7 +462,8 @@ FileCache<FileType>::FileCache(const string& cache_name,
cache_(NewLRUCache(DRAM_CACHE, max_open_files, cache_name)),
running_(1) {
if (entity) {
- cache_->SetMetrics(entity);
+ unique_ptr<FileCacheMetrics> metrics(new FileCacheMetrics(entity));
+ cache_->SetMetrics(std::move(metrics));
}
LOG(INFO) << Substitute("Constructed file cache $0 with capacity $1",
cache_name, max_open_files);
diff --git a/src/kudu/util/file_cache_metrics.cc
b/src/kudu/util/file_cache_metrics.cc
new file mode 100644
index 0000000..befdda6
--- /dev/null
+++ b/src/kudu/util/file_cache_metrics.cc
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/util/file_cache_metrics.h"
+
+#include "kudu/util/metrics.h"
+
+METRIC_DEFINE_counter(server, file_cache_inserts,
+ "File Cache Inserts", kudu::MetricUnit::kEntries,
+ "Number of file descriptors inserted in the cache");
+METRIC_DEFINE_counter(server, file_cache_lookups,
+ "File Cache Lookups", kudu::MetricUnit::kEntries,
+ "Number of file descriptors looked up from the cache");
+METRIC_DEFINE_counter(server, file_cache_evictions,
+ "File Cache Evictions", kudu::MetricUnit::kEntries,
+ "Number of file descriptors evicted from the cache");
+METRIC_DEFINE_counter(server, file_cache_misses,
+ "File Cache Misses", kudu::MetricUnit::kEntries,
+ "Number of lookups that didn't yield a file descriptor");
+METRIC_DEFINE_counter(server, file_cache_misses_caching,
+ "File Cache Misses (Caching)",
kudu::MetricUnit::kEntries,
+ "Number of lookups that were expecting a file descriptor
"
+ "that didn't yield one. Use this number instead of "
+ "cache_misses when trying to determine how "
+ "efficient the cache is");
+METRIC_DEFINE_counter(server, file_cache_hits,
+ "File Cache Hits", kudu::MetricUnit::kEntries,
+ "Number of lookups that found a file descriptor");
+METRIC_DEFINE_counter(server, file_cache_hits_caching,
+ "File Cache Hits (Caching)", kudu::MetricUnit::kEntries,
+ "Number of lookups that were expecting a file descriptor
"
+ "that found one. Use this number instead of cache_hits "
+ "when trying to determine how efficient the cache is");
+
+METRIC_DEFINE_gauge_uint64(server, file_cache_usage, "File Cache Usage",
+ kudu::MetricUnit::kEntries,
+ "Number of entries in the file cache");
+
+namespace kudu {
+
+#define MINIT(member, x) member = METRIC_##x.Instantiate(entity)
+#define GINIT(member, x) member = METRIC_##x.Instantiate(entity, 0)
+FileCacheMetrics::FileCacheMetrics(const scoped_refptr<MetricEntity>& entity) {
+ MINIT(inserts, file_cache_inserts);
+ MINIT(lookups, file_cache_lookups);
+ MINIT(evictions, file_cache_evictions);
+ MINIT(cache_hits, file_cache_hits);
+ MINIT(cache_hits_caching, file_cache_hits_caching);
+ MINIT(cache_misses, file_cache_misses);
+ MINIT(cache_misses_caching, file_cache_misses_caching);
+ GINIT(cache_usage, file_cache_usage);
+}
+#undef MINIT
+#undef GINIT
+
+} // namespace kudu
diff --git a/src/kudu/util/cache_metrics.h b/src/kudu/util/file_cache_metrics.h
similarity index 60%
copy from src/kudu/util/cache_metrics.h
copy to src/kudu/util/file_cache_metrics.h
index 04a546b..d16651c 100644
--- a/src/kudu/util/cache_metrics.h
+++ b/src/kudu/util/file_cache_metrics.h
@@ -14,29 +14,18 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_UTIL_CACHE_METRICS_H
-#define KUDU_UTIL_CACHE_METRICS_H
-#include <cstdint>
+#pragma once
#include "kudu/gutil/ref_counted.h"
-#include "kudu/util/metrics.h"
+#include "kudu/util/cache_metrics.h"
namespace kudu {
-struct CacheMetrics {
- explicit CacheMetrics(const scoped_refptr<MetricEntity>& metric_entity);
+class MetricEntity;
- scoped_refptr<Counter> inserts;
- scoped_refptr<Counter> lookups;
- scoped_refptr<Counter> evictions;
- scoped_refptr<Counter> cache_hits;
- scoped_refptr<Counter> cache_hits_caching;
- scoped_refptr<Counter> cache_misses;
- scoped_refptr<Counter> cache_misses_caching;
-
- scoped_refptr<AtomicGauge<uint64_t> > cache_usage;
+struct FileCacheMetrics : public CacheMetrics {
+ explicit FileCacheMetrics(const scoped_refptr<MetricEntity>& entity);
};
} // namespace kudu
-#endif /* KUDU_UTIL_CACHE_METRICS_H */
diff --git a/src/kudu/util/nvm_cache.cc b/src/kudu/util/nvm_cache.cc
index ef52ec0..bf450b9 100644
--- a/src/kudu/util/nvm_cache.cc
+++ b/src/kudu/util/nvm_cache.cc
@@ -26,14 +26,15 @@
#include <memory>
#include <mutex>
#include <string>
+#include <utility>
#include <vector>
#include <gflags/gflags.h>
#include <glog/logging.h>
#include <libvmem.h>
-#include "kudu/gutil/atomicops.h"
#include "kudu/gutil/atomic_refcount.h"
+#include "kudu/gutil/atomicops.h"
#include "kudu/gutil/dynamic_annotations.h"
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/gutil/hash/city.h"
@@ -65,15 +66,14 @@ DEFINE_bool(nvm_cache_simulate_allocation_failure, false,
"for testing.");
TAG_FLAG(nvm_cache_simulate_allocation_failure, unsafe);
+using std::string;
+using std::unique_ptr;
+using std::vector;
namespace kudu {
namespace {
-using std::shared_ptr;
-using std::string;
-using std::vector;
-
typedef simple_spinlock MutexType;
// LRU cache implementation
@@ -461,7 +461,7 @@ static const int kNumShards = 1 << kNumShardBits;
class ShardedLRUCache : public Cache {
private:
- gscoped_ptr<CacheMetrics> metrics_;
+ unique_ptr<CacheMetrics> metrics_;
vector<NvmLRUCache*> shards_;
VMEM* vmp_;
@@ -518,8 +518,8 @@ class ShardedLRUCache : public Cache {
return reinterpret_cast<LRUHandle*>(handle)->val_ptr();
}
- virtual void SetMetrics(const scoped_refptr<MetricEntity>& entity) OVERRIDE {
- metrics_.reset(new CacheMetrics(entity));
+ virtual void SetMetrics(unique_ptr<CacheMetrics> metrics) OVERRIDE {
+ metrics_ = std::move(metrics);
for (NvmLRUCache* cache : shards_) {
cache->SetMetrics(metrics_.get());
}
