You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/05/18 15:57:34 UTC
[2/5] kudu git commit: KUDU-2017. Fix calculation of perf improvement
for flushes
KUDU-2017. Fix calculation of perf improvement for flushes
The code to calculate the MM "perf improvement score" for a flush had two bugs:
(1) we calculated threshold - current_usage instead of current_usage -
threshold, which resulted in a negative score.
(2) we had an unsigned integer overflow, which resulted in the above negative
score becoming insanely large.
These two wrongs "made a right" in which we'd still trigger flushes at the
flush threshold, which is why this went unnoticed for quite some time. However,
the flushing behavior is more aggressive than originally intended, and we would
lose the correct prioritization of flushing tablets that are farther above the
threshold.
This patch addresses the issue.
I tested using tpch_real_world against a server configured with a 40G
memory limit. I verified the 'perf improvement' just prior to a flush
was the expected value (151 perf improvement listed for a tablet with
1.15G MRS and 1G flush threshold)
Change-Id: I8bcf443db857ce172b8e83758eb73a24c4b0c6af
Reviewed-on: http://gerrit.cloudera.org:8080/6908
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/e1104e4b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/e1104e4b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/e1104e4b
Branch: refs/heads/master
Commit: e1104e4b8e64459d6a90c24d41ccc764d377c85c
Parents: 1c5d91b
Author: Todd Lipcon <to...@cloudera.com>
Authored: Tue May 16 17:08:20 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu May 18 05:04:14 2017 +0000
----------------------------------------------------------------------
src/kudu/tablet/tablet_replica-test.cc | 4 ++--
src/kudu/tablet/tablet_replica_mm_ops.cc | 9 +++++----
2 files changed, 7 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/e1104e4b/src/kudu/tablet/tablet_replica-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_replica-test.cc b/src/kudu/tablet/tablet_replica-test.cc
index c48da6a..1a02b61 100644
--- a/src/kudu/tablet/tablet_replica-test.cc
+++ b/src/kudu/tablet/tablet_replica-test.cc
@@ -547,10 +547,10 @@ TEST_F(TabletReplicaTest, TestFlushOpsPerfImprovements) {
ASSERT_GT(stats.perf_improvement(), 0.01);
stats.Clear();
- // Way over the threshold, number is much higher than 1.
+ // Over the threshold, we expect improvement equal to the excess MB.
stats.set_ram_anchored(128 * 1024 * 1024);
FlushOpPerfImprovementPolicy::SetPerfImprovementForFlush(&stats, 1);
- ASSERT_LT(1.0, stats.perf_improvement());
+ ASSERT_NEAR(stats.perf_improvement(), 64, 0.01);
stats.Clear();
// Below the threshold but have been there a long time, closing in to 1.0.
http://git-wip-us.apache.org/repos/asf/kudu/blob/e1104e4b/src/kudu/tablet/tablet_replica_mm_ops.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_replica_mm_ops.cc b/src/kudu/tablet/tablet_replica_mm_ops.cc
index 85650ff..f2d16e2 100644
--- a/src/kudu/tablet/tablet_replica_mm_ops.cc
+++ b/src/kudu/tablet/tablet_replica_mm_ops.cc
@@ -65,15 +65,16 @@ const double kFlushUpperBoundMs = 60 * 60 * 1000;
void FlushOpPerfImprovementPolicy::SetPerfImprovementForFlush(MaintenanceOpStats* stats,
double elapsed_ms) {
- if (stats->ram_anchored() > FLAGS_flush_threshold_mb * 1024 * 1024) {
+ double anchored_mb = static_cast<double>(stats->ram_anchored()) / (1024 * 1024);
+ if (anchored_mb > FLAGS_flush_threshold_mb) {
// If we're over the user-specified flush threshold, then consider the perf
// improvement to be 1 for every extra MB. This produces perf_improvement results
- // which are much higher than any compaction would produce, and means that, when
+ // which are much higher than most compactions would produce, and means that, when
// there is an MRS over threshold, a flush will almost always be selected instead of
// a compaction. That's not necessarily a good thing, but in the absence of better
// heuristics, it will do for now.
- double extra_mb =
- static_cast<double>(FLAGS_flush_threshold_mb - (stats->ram_anchored()) / (1024 * 1024));
+ double extra_mb = anchored_mb - static_cast<double>(FLAGS_flush_threshold_mb);
+ DCHECK_GE(extra_mb, 0);
stats->set_perf_improvement(extra_mb);
} else if (elapsed_ms > FLAGS_flush_threshold_secs * 1000) {
// Even if we aren't over the threshold, consider flushing if we haven't flushed