Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [PATCH 03/11] vinyl: do not try to trigger dump if it is already in progress
Date: Thu, 20 Sep 2018 12:34:08 +0300	[thread overview]
Message-ID: <26cb12b606db996db31fb93b142938ca76e08701.1537435404.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1537435404.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1537435404.git.vdavydov.dev@gmail.com>

Currently, vy_quota_use() calls quota_exceeded_cb callback every time it
sees that the memory usage exceeds the watermark. Actually, this is
pointless, because the callback will return immediately if dump is
already in progress. Let's introduce flag vy_quota::dump_in_progress and
set it when dump is triggered so that we can skip callback invocation if
the flag is already set. The flag is cleared by vy_quota_dump(), which
is called when dump is complete. Since the callback is now used solely
to trigger dump, let's rename it to trigger_dump_cb.
---
 src/box/vinyl.c    |  9 +++++----
 src/box/vy_quota.c | 46 ++++++++++++++++++++++++++++++++++------------
 src/box/vy_quota.h | 24 ++++++++++++++++--------
 3 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 6cc5485b..d0262a03 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2436,8 +2436,8 @@ vinyl_engine_rollback_statement(struct engine *engine, struct txn *txn,
 
 /** {{{ Environment */
 
-static void
-vy_env_quota_exceeded_cb(struct vy_quota *quota)
+static int
+vy_env_trigger_dump_cb(struct vy_quota *quota)
 {
 	struct vy_env *env = container_of(quota, struct vy_env, quota);
 
@@ -2448,9 +2448,10 @@ vy_env_quota_exceeded_cb(struct vy_quota *quota)
 		 * quota has been consumed by pending transactions.
 		 * There's nothing we can do about that.
 		 */
-		return;
+		return -1;
 	}
 	vy_scheduler_trigger_dump(&env->scheduler);
+	return 0;
 }
 
 static void
@@ -2519,7 +2520,7 @@ vy_env_new(const char *path, size_t memory,
 			      vy_squash_schedule, e) != 0)
 		goto error_lsm_env;
 
-	if (vy_quota_create(&e->quota, vy_env_quota_exceeded_cb) != 0)
+	if (vy_quota_create(&e->quota, vy_env_trigger_dump_cb) != 0)
 		goto error_quota;
 
 	struct slab_cache *slab_cache = cord_slab_cache();
diff --git a/src/box/vy_quota.c b/src/box/vy_quota.c
index 8ead5c4b..852f381b 100644
--- a/src/box/vy_quota.c
+++ b/src/box/vy_quota.c
@@ -68,6 +68,29 @@ static const size_t VY_DEFAULT_DUMP_BANDWIDTH = 10 * 1024 * 1024;
  */
 enum { VY_DUMP_BANDWIDTH_PCT = 10 };
 
+/**
+ * Trigger memory dump unless it is already in progress.
+ */
+static void
+vy_quota_trigger_dump(struct vy_quota *q)
+{
+	if (q->dump_in_progress)
+		return;
+	if (q->trigger_dump_cb(q) != 0)
+		return;
+	q->dump_in_progress = true;
+}
+
+/**
+ * Trigger memory dump if usage is above the watermark.
+ */
+static void
+vy_quota_check_watermark(struct vy_quota *q)
+{
+	if (q->used >= q->watermark)
+		vy_quota_trigger_dump(q);
+}
+
 static void
 vy_quota_timer_cb(ev_loop *loop, ev_timer *timer, int events)
 {
@@ -98,13 +121,14 @@ vy_quota_timer_cb(ev_loop *loop, ev_timer *timer, int events)
 	 */
 	q->watermark = ((double)q->limit * q->dump_bw /
 			(q->dump_bw + q->use_rate + 1));
-	if (q->used >= q->watermark)
-		q->quota_exceeded_cb(q);
+	vy_quota_check_watermark(q);
 }
 
 int
-vy_quota_create(struct vy_quota *q, vy_quota_exceeded_f quota_exceeded_cb)
+vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb)
 {
+	memset(q, 0, sizeof(*q));
+
 	enum { KB = 1024, MB = KB * KB };
 	static int64_t dump_bandwidth_buckets[] = {
 		100 * KB, 200 * KB, 300 * KB, 400 * KB, 500 * KB, 600 * KB,
@@ -127,12 +151,9 @@ vy_quota_create(struct vy_quota *q, vy_quota_exceeded_f quota_exceeded_cb)
 
 	q->limit = SIZE_MAX;
 	q->watermark = SIZE_MAX;
-	q->used = 0;
-	q->use_curr = 0;
-	q->use_rate = 0;
 	q->too_long_threshold = TIMEOUT_INFINITY;
 	q->dump_bw = VY_DEFAULT_DUMP_BANDWIDTH;
-	q->quota_exceeded_cb = quota_exceeded_cb;
+	q->trigger_dump_cb = trigger_dump_cb;
 	fiber_cond_create(&q->cond);
 	ev_timer_init(&q->timer, vy_quota_timer_cb, 0,
 		      VY_QUOTA_UPDATE_INTERVAL);
@@ -154,14 +175,15 @@ void
 vy_quota_set_limit(struct vy_quota *q, size_t limit)
 {
 	q->limit = q->watermark = limit;
-	if (q->used >= limit)
-		q->quota_exceeded_cb(q);
+	vy_quota_check_watermark(q);
 	fiber_cond_signal(&q->cond);
 }
 
 void
 vy_quota_dump(struct vy_quota *q, size_t size, double duration)
 {
+	q->dump_in_progress = false;
+
 	/*
 	 * Release quota and wake up the first fiber in
 	 * the wait queue, if any.
@@ -195,8 +217,7 @@ vy_quota_force_use(struct vy_quota *q, size_t size)
 {
 	q->used += size;
 	q->use_curr += size;
-	if (q->used >= q->watermark)
-		q->quota_exceeded_cb(q);
+	vy_quota_check_watermark(q);
 }
 
 /**
@@ -218,7 +239,8 @@ vy_quota_use(struct vy_quota *q, size_t size, double timeout)
 		double deadline = start_time + timeout;
 
 		do {
-			q->quota_exceeded_cb(q);
+			if (q->used + size > q->limit)
+				vy_quota_trigger_dump(q);
 			if (fiber_cond_wait_deadline(&q->cond, deadline) != 0)
 				return -1; /* timed out */
 		} while (!vy_quota_may_use(q, size));
diff --git a/src/box/vy_quota.h b/src/box/vy_quota.h
index 0fa852b1..3b020829 100644
--- a/src/box/vy_quota.h
+++ b/src/box/vy_quota.h
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 
+#include <stdbool.h>
 #include <stddef.h>
 #include <tarantool_ev.h>
 #include "fiber_cond.h"
@@ -42,8 +43,8 @@ extern "C" {
 struct vy_quota;
 struct histogram;
 
-typedef void
-(*vy_quota_exceeded_f)(struct vy_quota *quota);
+typedef int
+(*vy_quota_trigger_dump_f)(struct vy_quota *quota);
 
 /**
  * Quota used for accounting and limiting memory consumption
@@ -73,11 +74,6 @@ struct vy_quota {
 	 * there is no quota left.
 	 */
 	struct fiber_cond cond;
-	/**
-	 * Called when quota is consumed if used >= watermark.
-	 * It is supposed to trigger memory reclaim.
-	 */
-	vy_quota_exceeded_f quota_exceeded_cb;
 	/** Timer for updating quota watermark. */
 	ev_timer timer;
 	/**
@@ -91,6 +87,18 @@ struct vy_quota {
 	 * moving average of use_curr.
 	 */
 	size_t use_rate;
+	/**
+	 * Called when quota is consumed if used >= watermark.
+	 * It is supposed to trigger memory dump and return 0
+	 * on success or -1 on failure.
+	 */
+	vy_quota_trigger_dump_f trigger_dump_cb;
+	/**
+	 * Set if the last triggered memory dump hasn't completed
+	 * yet, i.e. trigger_dump_cb() was successfully invoked,
+	 * but quota hasn't been released yet.
+	 */
+	bool dump_in_progress;
 	/** Current dump bandwidth estimate. */
 	size_t dump_bw;
 	/**
@@ -108,7 +116,7 @@ struct vy_quota {
 };
 
 int
-vy_quota_create(struct vy_quota *q, vy_quota_exceeded_f quota_exceeded_cb);
+vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb);
 
 void
 vy_quota_destroy(struct vy_quota *q);
-- 
2.11.0

  parent reply	other threads:[~2018-09-20  9:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20  9:34 [PATCH 00/11] vinyl: prepare for transaction throttling Vladimir Davydov
2018-09-20  9:34 ` [PATCH 01/11] vinyl: merge vy_quota_release and vy_quota_update_dump_bandwidth Vladimir Davydov
2018-09-20  9:34 ` [PATCH 02/11] vinyl: refactor quota use/unuse methods Vladimir Davydov
2018-09-20  9:34 ` Vladimir Davydov [this message]
2018-09-20  9:34 ` [PATCH 04/11] vinyl: don't start quota timer until local recovery is complete Vladimir Davydov
2018-09-20  9:34 ` [PATCH 05/11] vinyl: add helper to start scheduler and enable quota on startup Vladimir Davydov
2018-09-25 23:22   ` [tarantool-patches] " Konstantin Osipov
2018-09-20  9:34 ` [PATCH 06/11] vinyl: zap vy_env::memory, read_threads, and write_threads Vladimir Davydov
2018-09-25 23:23   ` [tarantool-patches] " Konstantin Osipov
2018-09-20  9:34 ` [PATCH 07/11] vinyl: do not account zero dump bandwidth Vladimir Davydov
2018-09-25 23:24   ` [tarantool-patches] " Konstantin Osipov
2018-09-20  9:34 ` [PATCH 08/11] vinyl: set quota timer period to 100 ms Vladimir Davydov
2018-09-20  9:34 ` [PATCH 09/11] vinyl: implement basic transaction throttling Vladimir Davydov
2018-09-20  9:34 ` [PATCH 10/11] vinyl: implement quota wait queue without fiber_cond Vladimir Davydov
2018-09-20  9:34 ` [PATCH 11/11] vinyl: split quota consumption rate limit into soft and hard Vladimir Davydov
2018-09-25 23:19 ` [tarantool-patches] Re: [PATCH 00/11] vinyl: prepare for transaction throttling Konstantin Osipov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26cb12b606db996db31fb93b142938ca76e08701.1537435404.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 03/11] vinyl: do not try to trigger dump if it is already in progress' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox