You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2018/04/30 15:31:54 UTC
[1/4] kudu git commit: cache: reduce contention on
MemTracker::Release and Consume
Repository: kudu
Updated Branches:
refs/heads/master 08287b032 -> eef20da65
cache: reduce contention on MemTracker::Release and Consume
In an old benchmark about a year ago, this improved YCSB load throughput
from ~450k ops/sec to ~570k. For the newly-added cache-bench, this
improves performance substantially (16x) for the scenario with high churn:
Before:
[ RUN ] Patterns/CacheBench.RunBench/3
I0425 13:59:17.307648 95430 cache-bench.cc:176] Warming up...
I0425 13:59:18.308992 95430 cache-bench.cc:179] Running benchmark...
I0425 13:59:19.313868 95430 cache-bench.cc:187] UNIFORM ratio=3.00x n_unique=786432: 2.48M lookups/sec
I0425 13:59:19.313891 95430 cache-bench.cc:188] UNIFORM ratio=3.00x n_unique=786432: 33.3% hit rate
[ OK ] Patterns/CacheBench.RunBench/3 (2192 ms)
After:
[ RUN ] Patterns/CacheBench.RunBench/3
I0425 13:57:40.581399 94314 cache-bench.cc:176] Warming up...
I0425 13:57:41.582798 94314 cache-bench.cc:179] Running benchmark...
I0425 13:57:42.583904 94314 cache-bench.cc:187] UNIFORM ratio=3.00x n_unique=786432: 39.53M lookups/sec
I0425 13:57:42.583930 94314 cache-bench.cc:188] UNIFORM ratio=3.00x n_unique=786432: 33.3% hit rate
[ OK ] Patterns/CacheBench.RunBench/3 (2055 ms)
Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
Reviewed-on: http://gerrit.cloudera.org:8080/6597
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/64d6d38b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/64d6d38b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/64d6d38b
Branch: refs/heads/master
Commit: 64d6d38ba101b08506677ca0e8c2ffafec0b1459
Parents: 08287b0
Author: Todd Lipcon <to...@apache.org>
Authored: Fri Apr 7 16:48:44 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Apr 27 19:59:23 2018 +0000
----------------------------------------------------------------------
src/kudu/cfile/block_cache-test.cc | 10 ++++++-
src/kudu/util/cache-test.cc | 6 ++++
src/kudu/util/cache.cc | 53 +++++++++++++++++++++++++++++++--
3 files changed, 65 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/64d6d38b/src/kudu/cfile/block_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/block_cache-test.cc b/src/kudu/cfile/block_cache-test.cc
index b34f0b1..5caebce 100644
--- a/src/kudu/cfile/block_cache-test.cc
+++ b/src/kudu/cfile/block_cache-test.cc
@@ -15,24 +15,32 @@
// specific language governing permissions and limitations
// under the License.
+#include "kudu/cfile/block_cache.h"
+
#include <cstring>
#include <memory>
#include <ostream>
+#include <gflags/gflags_declare.h>
#include <glog/logging.h>
#include <gtest/gtest.h>
-#include "kudu/cfile/block_cache.h"
#include "kudu/util/cache.h"
#include "kudu/util/mem_tracker.h"
#include "kudu/util/slice.h"
+DECLARE_double(cache_memtracker_approximation_ratio);
+
namespace kudu {
namespace cfile {
static const char *DATA_TO_CACHE = "hello world";
TEST(TestBlockCache, TestBasics) {
+ // Disable approximate tracking of cache memory since we make specific
+ // assertions on the MemTracker in this test.
+ FLAGS_cache_memtracker_approximation_ratio = 0;
+
size_t data_size = strlen(DATA_TO_CACHE) + 1;
BlockCache cache(512 * 1024 * 1024);
BlockCache::FileId id(1234);
http://git-wip-us.apache.org/repos/asf/kudu/blob/64d6d38b/src/kudu/util/cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/cache-test.cc b/src/kudu/util/cache-test.cc
index 6baa215..a72a871 100644
--- a/src/kudu/util/cache-test.cc
+++ b/src/kudu/util/cache-test.cc
@@ -30,6 +30,8 @@
DECLARE_string(nvm_cache_path);
#endif // defined(__linux__)
+DECLARE_double(cache_memtracker_approximation_ratio);
+
namespace kudu {
// Conversions between numeric keys/values and the types expected by Cache.
@@ -70,6 +72,10 @@ class CacheTest : public KuduTest,
}
#endif // defined(__linux__)
+ // Disable approximate tracking of cache memory since we make specific
+ // assertions on the MemTracker in this test.
+ FLAGS_cache_memtracker_approximation_ratio = 0;
+
cache_.reset(NewLRUCache(GetParam(), kCacheSize, "cache_test"));
MemTracker::FindTracker("cache_test-sharded_lru_cache", &mem_tracker_);
http://git-wip-us.apache.org/repos/asf/kudu/blob/64d6d38b/src/kudu/util/cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/cache.cc b/src/kudu/util/cache.cc
index 1fe11b2..bd6fa05 100644
--- a/src/kudu/util/cache.cc
+++ b/src/kudu/util/cache.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <atomic>
#include <cstdint>
#include <cstring>
#include <memory>
@@ -45,6 +46,12 @@ DEFINE_bool(cache_force_single_shard, false,
"Override all cache implementations to use just one shard");
TAG_FLAG(cache_force_single_shard, hidden);
+DEFINE_double(cache_memtracker_approximation_ratio, 0.01,
+ "The MemTracker associated with a cache can accumulate error up to "
+ "this ratio to improve performance. For tests.");
+TAG_FLAG(cache_memtracker_approximation_ratio, hidden);
+
+using std::atomic;
using std::shared_ptr;
using std::string;
using std::vector;
@@ -188,7 +195,10 @@ class LRUCache {
~LRUCache();
// Separate from constructor so caller can easily make an array of LRUCache
- void SetCapacity(size_t capacity) { capacity_ = capacity; }
+ void SetCapacity(size_t capacity) {
+ capacity_ = capacity;
+ max_deferred_consumption_ = capacity * FLAGS_cache_memtracker_approximation_ratio;
+ }
void SetMetrics(CacheMetrics* metrics) { metrics_ = metrics; }
@@ -207,6 +217,26 @@ class LRUCache {
// Call the user's eviction callback, if it exists, and free the entry.
void FreeEntry(LRUHandle* e);
+ // Update the memtracker's consumption by the given amount.
+ //
+ // This "buffers" the updates locally in 'deferred_consumption_' until the amount
+ // of accumulated delta is more than ~1% of the cache capacity. This improves
+ // performance under workloads with high eviction rates for a few reasons:
+ //
+ // 1) once the cache reaches its full capacity, we expect it to remain there
+ // in steady state. Each insertion is usually matched by an eviction, and unless
+ // the total size of the evicted item(s) is much different than the size of the
+ // inserted item, each eviction event is unlikely to change the total cache usage
+ // much. So, we expect that the accumulated error will mostly remain around 0
+ // and we can avoid propagating changes to the MemTracker at all.
+ //
+ // 2) because the cache implementation is sharded, we do this tracking in a bunch
+ // of different locations, avoiding bouncing cache-lines between cores. By contrast
+ // the MemTracker is a simple integer, so it doesn't scale as well under concurrency.
+ //
+ // Positive delta indicates an increased memory consumption.
+ void UpdateMemTracker(int64_t delta);
+
// Initialized before use.
size_t capacity_;
@@ -221,6 +251,11 @@ class LRUCache {
HandleTable table_;
MemTracker* mem_tracker_;
+ atomic<int64_t> deferred_consumption_ { 0 };
+
+ // Initialized based on capacity_ to ensure an upper bound on the error on the
+ // MemTracker consumption.
+ int64_t max_deferred_consumption_;
CacheMetrics* metrics_;
};
@@ -243,6 +278,7 @@ LRUCache::~LRUCache() {
}
e = next;
}
+ mem_tracker_->Consume(deferred_consumption_);
}
bool LRUCache::Unref(LRUHandle* e) {
@@ -255,7 +291,7 @@ void LRUCache::FreeEntry(LRUHandle* e) {
if (e->eviction_callback) {
e->eviction_callback->EvictedEntry(e->key(), e->value());
}
- mem_tracker_->Release(e->charge);
+ UpdateMemTracker(-static_cast<int64_t>(e->charge));
if (PREDICT_TRUE(metrics_)) {
metrics_->cache_usage->DecrementBy(e->charge);
metrics_->evictions->Increment();
@@ -263,6 +299,17 @@ void LRUCache::FreeEntry(LRUHandle* e) {
delete [] e;
}
+void LRUCache::UpdateMemTracker(int64_t delta) {
+ int64_t old_deferred = deferred_consumption_.fetch_add(delta);
+ int64_t new_deferred = old_deferred + delta;
+
+ if (new_deferred > max_deferred_consumption_ ||
+ new_deferred < -max_deferred_consumption_) {
+ int64_t to_propagate = deferred_consumption_.exchange(0, std::memory_order_relaxed);
+ mem_tracker_->Consume(to_propagate);
+ }
+}
+
void LRUCache::LRU_Remove(LRUHandle* e) {
e->next->prev = e->prev;
e->prev->next = e->next;
@@ -326,7 +373,7 @@ Cache::Handle* LRUCache::Insert(LRUHandle* e, Cache::EvictionCallback *eviction_
// Allocate().
e->eviction_callback = eviction_callback;
e->refs = 2; // One from LRUCache, one for the returned handle
- mem_tracker_->Consume(e->charge);
+ UpdateMemTracker(e->charge);
if (PREDICT_TRUE(metrics_)) {
metrics_->cache_usage->IncrementBy(e->charge);
metrics_->inserts->Increment();
[4/4] kudu git commit: Fix table formatting in full data dirs docs
Posted by al...@apache.org.
Fix table formatting in full data dirs docs
Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Reviewed-on: http://gerrit.cloudera.org:8080/10211
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/eef20da6
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/eef20da6
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/eef20da6
Branch: refs/heads/master
Commit: eef20da6506efabd8874f554e45b2477f3c8096c
Parents: f851748
Author: Will Berkeley <wd...@apache.org>
Authored: Mon Apr 30 07:44:29 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Mon Apr 30 15:29:48 2018 +0000
----------------------------------------------------------------------
docs/administration.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/eef20da6/docs/administration.adoc
----------------------------------------------------------------------
diff --git a/docs/administration.adoc b/docs/administration.adoc
index 828658f..dd175b4 100644
--- a/docs/administration.adoc
+++ b/docs/administration.adoc
@@ -825,7 +825,7 @@ The table below describes this behavior for each type of directory. The behavior
is uniform across masters and tablet servers.
[[disk_full_behavior]]
.Kudu Full Disk Behavior
-[cols="<,<,<",options="header"]
+[options="header"]
|===
| Kudu Directory Type | Crash on a Full Disk?
| Directory containing WALs | Yes
[2/4] kudu git commit: cache: switch to std::atomic
Posted by al...@apache.org.
cache: switch to std::atomic
A previous commit added some use of std::atomic, so this patch makes the
file self-consistent.
Change-Id: I359f273af9ba7130d230ef639a40e12a8ac51a06
Reviewed-on: http://gerrit.cloudera.org:8080/10209
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/0aa22030
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/0aa22030
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/0aa22030
Branch: refs/heads/master
Commit: 0aa220303276af8979448507c74e3e2820db6fbc
Parents: 64d6d38
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Apr 25 14:26:45 2018 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Apr 27 19:59:30 2018 +0000
----------------------------------------------------------------------
src/kudu/util/cache.cc | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/0aa22030/src/kudu/util/cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/cache.cc b/src/kudu/util/cache.cc
index bd6fa05..c0bbba3 100644
--- a/src/kudu/util/cache.cc
+++ b/src/kudu/util/cache.cc
@@ -2,6 +2,8 @@
// 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 <atomic>
#include <cstdint>
#include <cstring>
@@ -14,12 +16,9 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include "kudu/gutil/atomicops.h"
-#include "kudu/gutil/atomic_refcount.h"
-#include "kudu/gutil/dynamic_annotations.h"
#include "kudu/gutil/bits.h"
-#include "kudu/gutil/hash/city.h"
#include "kudu/gutil/gscoped_ptr.h"
+#include "kudu/gutil/hash/city.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
@@ -27,7 +26,6 @@
#include "kudu/gutil/strings/substitute.h"
#include "kudu/gutil/sysinfo.h"
#include "kudu/util/alignment.h"
-#include "kudu/util/cache.h"
#include "kudu/util/cache_metrics.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/locks.h"
@@ -77,7 +75,7 @@ struct LRUHandle {
size_t charge; // TODO(opt): Only allow uint32_t?
uint32_t key_length;
uint32_t val_length;
- Atomic32 refs;
+ std::atomic<int32_t> refs;
uint32_t hash; // Hash of key(); used for fast sharding and comparisons
// The storage for the key/value pair itself. The data is stored as:
@@ -272,7 +270,8 @@ LRUCache::LRUCache(MemTracker* tracker)
LRUCache::~LRUCache() {
for (LRUHandle* e = lru_.next; e != &lru_; ) {
LRUHandle* next = e->next;
- DCHECK_EQ(e->refs, 1); // Error if caller has an unreleased handle
+ DCHECK_EQ(e->refs.load(std::memory_order_relaxed), 1)
+ << "caller has an unreleased handle";
if (Unref(e)) {
FreeEntry(e);
}
@@ -282,12 +281,12 @@ LRUCache::~LRUCache() {
}
bool LRUCache::Unref(LRUHandle* e) {
- DCHECK_GT(ANNOTATE_UNPROTECTED_READ(e->refs), 0);
- return !base::RefCountDec(&e->refs);
+ DCHECK_GT(e->refs.load(std::memory_order_relaxed), 0);
+ return e->refs.fetch_sub(1) == 1;
}
void LRUCache::FreeEntry(LRUHandle* e) {
- DCHECK_EQ(ANNOTATE_UNPROTECTED_READ(e->refs), 0);
+ DCHECK_EQ(e->refs.load(std::memory_order_relaxed), 0);
if (e->eviction_callback) {
e->eviction_callback->EvictedEntry(e->key(), e->value());
}
@@ -331,7 +330,7 @@ Cache::Handle* LRUCache::Lookup(const Slice& key, uint32_t hash, bool caching) {
std::lock_guard<MutexType> l(mutex_);
e = table_.Lookup(key, hash);
if (e != nullptr) {
- base::RefCountInc(&e->refs);
+ e->refs.fetch_add(1, std::memory_order_relaxed);
LRU_Remove(e);
LRU_Append(e);
}
@@ -372,7 +371,7 @@ Cache::Handle* LRUCache::Insert(LRUHandle* e, Cache::EvictionCallback *eviction_
// Set the remaining LRUHandle members which were not already allocated during
// Allocate().
e->eviction_callback = eviction_callback;
- e->refs = 2; // One from LRUCache, one for the returned handle
+ e->refs.store(2, std::memory_order_relaxed); // One from LRUCache, one for the returned handle
UpdateMemTracker(e->charge);
if (PREDICT_TRUE(metrics_)) {
metrics_->cache_usage->IncrementBy(e->charge);
[3/4] kudu git commit: [Java] Use spotbugs in place of findbugs in
gradle
Posted by al...@apache.org.
[Java] Use spotbugs in place of findbugs in gradle
Change-Id: I2c9863dc59cc6670e2f25c857ba8662b5d4fe4e6
Reviewed-on: http://gerrit.cloudera.org:8080/10187
Tested-by: Grant Henke <gr...@apache.org>
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f8517483
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f8517483
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f8517483
Branch: refs/heads/master
Commit: f8517483935358aa6f7cf21b11717102476ab87d
Parents: 0aa2203
Author: Grant Henke <gr...@apache.org>
Authored: Wed Apr 25 11:39:30 2018 -0500
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Apr 27 21:55:59 2018 +0000
----------------------------------------------------------------------
java/buildSrc/build.gradle | 1 +
java/gradle/dependencies.gradle | 2 +-
java/gradle/quality.gradle | 16 ++++++++--------
3 files changed, 10 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/f8517483/java/buildSrc/build.gradle
----------------------------------------------------------------------
diff --git a/java/buildSrc/build.gradle b/java/buildSrc/build.gradle
index 8e380fa..d69e733 100644
--- a/java/buildSrc/build.gradle
+++ b/java/buildSrc/build.gradle
@@ -33,6 +33,7 @@ dependencies {
compile "com.github.jengelman.gradle.plugins:shadow:2.0.3"
compile "com.google.protobuf:protobuf-gradle-plugin:0.8.5"
compile "com.netflix.nebula:nebula-clojure-plugin:5.0.6"
+ compile "gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:1.6.1"
compile "io.spring.gradle:propdeps-plugin:0.0.9.RELEASE"
compile "net.ltgt.gradle:gradle-errorprone-plugin:0.0.13"
compile "ru.vyarus:gradle-animalsniffer-plugin:1.4.3"
http://git-wip-us.apache.org/repos/asf/kudu/blob/f8517483/java/gradle/dependencies.gradle
----------------------------------------------------------------------
diff --git a/java/gradle/dependencies.gradle b/java/gradle/dependencies.gradle
index b6980e4..28dfd51 100755
--- a/java/gradle/dependencies.gradle
+++ b/java/gradle/dependencies.gradle
@@ -31,7 +31,6 @@ versions += [
clojureToolsCli: "0.3.5",
commonsIo : "2.6",
errorprone : "2.3.1",
- findbugs : "3.0.1",
flume : "1.8.0",
gradle : "4.7",
// We use the android version instead of the jre version
@@ -59,6 +58,7 @@ versions += [
slf4j : "1.7.25",
spark : "2.3.0",
sparkAvro : "4.0.0",
+ spotBugs : "3.1.3",
yetus : "0.7.0"
]
http://git-wip-us.apache.org/repos/asf/kudu/blob/f8517483/java/gradle/quality.gradle
----------------------------------------------------------------------
diff --git a/java/gradle/quality.gradle b/java/gradle/quality.gradle
index de6fa05..b256edd 100644
--- a/java/gradle/quality.gradle
+++ b/java/gradle/quality.gradle
@@ -18,7 +18,7 @@
// This file contains common tasks and configuration for checking the quality of the code.
apply plugin: "checkstyle" // Ensures Java code follows the defined coding style.
-apply plugin: "findbugs" // Performs static code analysis to look for bugs in Java code.
+apply plugin: "com.github.spotbugs" // Performs static code analysis to look for bugs in Java code.
apply plugin: "net.ltgt.errorprone" // Performs static code analysis to look for bugs in Java code.
apply plugin: "pmd" // Performs static code analysis to look for common code smells in Java code.
apply plugin: "com.github.ben-manes.versions" // Provides a task to determine which dependencies have updates.
@@ -39,23 +39,23 @@ task checkstyle(dependsOn: [checkstyleMain, checkstyleTest, checkstyleIntegratio
description = "Run Checkstyle analysis."
}
-findbugs {
- toolVersion = versions.findbugs
+spotbugs {
+ toolVersion = versions.spotBugs
ignoreFailures = true
effort = "max"
}
-tasks.withType(FindBugs) {
+tasks.withType(com.github.spotbugs.SpotBugsTask) {
reports {
xml.enabled false
html.enabled true
}
}
-// Create an aggregate findbugs task.
-// This simplifies running findbugs on all the code by only needing one task instead of multiple in your command.
-task findbugs(dependsOn: [findbugsMain, findbugsTest, findbugsIntegrationTest], group: "Verification") {
- description = "Run FindBugs analysis."
+// Create an aggregate spotbugs task.
+// This simplifies running spotbugs on all the code by only needing one task instead of multiple in your command.
+task spotbugs(dependsOn: [spotbugsMain, spotbugsTest, spotbugsIntegrationTest], group: "Verification") {
+ description = "Run SpotBugs analysis."
}
pmd {