[PATCH 14/18] vinyl: do not try to trigger dump if it is already in progress

[Vladimir Davydov <vdavydov.dev@gmail.com>]

Currently, vy_quota_try_use() calls vy_quota_exceeded_cb() 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 a flag vy_quota::dump_in_progress
and skip the call to vy_quota_exceeded_cb() if dump has already been
triggered. The flag is cleared when dump by vy_quota_dump(), which is
called when dump is complete. This also gives us a place in the code
where we can remember the time and memory usage at the time when dump
was started, which will be useful for transaction throttling.
---
 src/box/vinyl.c    |  5 +++--
 src/box/vy_quota.c | 46 +++++++++++++++++++++++++++++++++-------------
 src/box/vy_quota.h | 12 ++++++++++--
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 3699fff8..25a937bc 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2366,7 +2366,7 @@ vinyl_engine_rollback_statement(struct engine *engine, struct txn *txn,
 
 /** {{{ Environment */
 
-static void
+static bool
 vy_env_quota_exceeded_cb(struct vy_quota *quota)
 {
 	struct vy_env *env = container_of(quota, struct vy_env, quota);
@@ -2393,9 +2393,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 false;
 	}
 	vy_scheduler_trigger_dump(&env->scheduler);
+	return true;
 }
 
 static void
diff --git a/src/box/vy_quota.c b/src/box/vy_quota.c
index 620155ab..0250b3ab 100644
--- a/src/box/vy_quota.c
+++ b/src/box/vy_quota.c
@@ -79,6 +79,17 @@ vy_quota_signal(struct vy_quota *q)
 		fiber_cond_signal(&q->cond);
 }
 
+/**
+ * Trigger memory dump if memory usage is above the wateramrk
+ * and dump hasn't been triggered yet.
+ */
+static inline void
+vy_quota_check_watermark(struct vy_quota *q)
+{
+	if (!q->dump_in_progress && q->used >= q->watermark)
+		q->dump_in_progress = q->quota_exceeded_cb(q);
+}
+
 static void
 vy_quota_timer_cb(ev_loop *loop, ev_timer *timer, int events)
 {
@@ -109,8 +120,7 @@ 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
@@ -144,6 +154,7 @@ vy_quota_create(struct vy_quota *q, vy_quota_exceeded_f quota_exceeded_cb)
 	q->too_long_threshold = TIMEOUT_INFINITY;
 	q->dump_bw = VY_DEFAULT_DUMP_BANDWIDTH;
 	q->quota_exceeded_cb = quota_exceeded_cb;
+	q->dump_in_progress = false;
 	fiber_cond_create(&q->cond);
 	ev_timer_init(&q->timer, vy_quota_timer_cb, 0,
 		      VY_QUOTA_UPDATE_INTERVAL);
@@ -165,8 +176,7 @@ 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);
 	vy_quota_signal(q);
 }
 
@@ -182,8 +192,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);
 }
 
 void
@@ -191,6 +200,7 @@ vy_quota_dump(struct vy_quota *q, size_t size, double duration)
 {
 	assert(q->used >= size);
 	q->used -= size;
+	q->dump_in_progress = false;
 	vy_quota_signal(q);
 
 	/* Account dump bandwidth. */
@@ -211,9 +221,14 @@ vy_quota_try_use(struct vy_quota *q, size_t size, double timeout)
 {
 	double start_time = ev_monotonic_now(loop());
 	double deadline = start_time + timeout;
-	while (q->used + size > q->limit && timeout > 0) {
-		q->quota_exceeded_cb(q);
-		if (fiber_cond_wait_deadline(&q->cond, deadline) != 0)
+
+	q->used += size;
+	while (q->used > q->limit) {
+		vy_quota_check_watermark(q);
+		q->used -= size;
+		int rc = fiber_cond_wait_deadline(&q->cond, deadline);
+		q->used += size;
+		if (rc != 0)
 			break; /* timed out */
 	}
 	double wait_time = ev_monotonic_now(loop()) - start_time;
@@ -221,12 +236,17 @@ vy_quota_try_use(struct vy_quota *q, size_t size, double timeout)
 		say_warn("waited for %zu bytes of vinyl memory quota "
 			 "for too long: %.3f sec", size, wait_time);
 	}
-	if (q->used + size > q->limit)
+	if (q->used > q->limit) {
+		/*
+		 * In case of faiure diag should have been set by
+		 * fiber_cond_wait_deadline().
+		 */
+		assert(!diag_is_empty(diag_get()));
+		q->used -= size;
 		return -1;
-	q->used += size;
+	}
 	q->use_curr += size;
-	if (q->used >= q->watermark)
-		q->quota_exceeded_cb(q);
+	vy_quota_check_watermark(q);
 
 	/* Wake up the next fiber in the line waiting for quota. */
 	vy_quota_signal(q);
diff --git a/src/box/vy_quota.h b/src/box/vy_quota.h
index 287c50f2..cb681386 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,7 +43,7 @@ extern "C" {
 struct vy_quota;
 struct histogram;
 
-typedef void
+typedef bool
 (*vy_quota_exceeded_f)(struct vy_quota *quota);
 
 /**
@@ -75,9 +76,16 @@ struct vy_quota {
 	struct fiber_cond cond;
 	/**
 	 * Called when quota is consumed if used >= watermark.
-	 * It is supposed to trigger memory reclaim.
+	 * It is supposed to trigger memory dump and return true
+	 * on success, false on failure.
 	 */
 	vy_quota_exceeded_f quota_exceeded_cb;
+	/**
+	 * Set if the last triggered memory dump hasn't completed
+	 * yet, i.e. quota_exceeded_cb() was invoked and returned
+	 * true, but vy_quota_dump() hasn't been called yet.
+	 */
+	bool dump_in_progress;
 	/** Timer for updating quota watermark. */
 	ev_timer timer;
 	/**
-- 
2.11.0