From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH 04/11] vinyl: don't start quota timer until local recovery is complete Date: Thu, 20 Sep 2018 12:34:09 +0300 [thread overview] Message-ID: <6f21c27a5f71c969dce68daffe4ef4d1f852e5ac.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> Although we don't impose the limit during local recovery, the quota timer, which keeps track of the write rate and recalculates the watermark, is running. This is incorrect, because the write rate can rocket sky high during recovery so that we can wind up with the watermark set too low once recovery is complete. When we introduce transaction throttling, the timer will also set a throttle limit based on the write rate, which is pointless during local recovery too. That said, we shouldn't start the quota timer until local recovery is complete. To do that, let's introduce new function vy_quota_enable() that enables the limit and starts the timer. The function is called instead of vy_quota_set_limit() upon bootstrap and recovery completion. --- src/box/vinyl.c | 8 ++++---- src/box/vy_quota.c | 25 ++++++++++++++++++++----- src/box/vy_quota.h | 25 ++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index d0262a03..e1b35f3a 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -2520,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_trigger_dump_cb) != 0) + if (vy_quota_create(&e->quota, e->memory, vy_env_trigger_dump_cb) != 0) goto error_quota; struct slab_cache *slab_cache = cord_slab_cache(); @@ -2723,7 +2723,7 @@ vinyl_engine_bootstrap(struct engine *engine) if (vy_log_bootstrap() != 0) return -1; vy_scheduler_start(&e->scheduler); - vy_quota_set_limit(&e->quota, e->memory); + vy_quota_enable(&e->quota); e->status = VINYL_ONLINE; return 0; } @@ -2758,7 +2758,7 @@ vinyl_engine_begin_initial_recovery(struct engine *engine, if (vy_log_bootstrap() != 0) return -1; vy_scheduler_start(&e->scheduler); - vy_quota_set_limit(&e->quota, e->memory); + vy_quota_enable(&e->quota); e->status = VINYL_INITIAL_RECOVERY_REMOTE; } return 0; @@ -2802,7 +2802,7 @@ vinyl_engine_end_recovery(struct engine *engine) e->recovery = NULL; e->recovery_vclock = NULL; vy_scheduler_start(&e->scheduler); - vy_quota_set_limit(&e->quota, e->memory); + vy_quota_enable(&e->quota); break; case VINYL_FINAL_RECOVERY_REMOTE: break; diff --git a/src/box/vy_quota.c b/src/box/vy_quota.c index 852f381b..7cd64474 100644 --- a/src/box/vy_quota.c +++ b/src/box/vy_quota.c @@ -74,6 +74,7 @@ enum { VY_DUMP_BANDWIDTH_PCT = 10 }; static void vy_quota_trigger_dump(struct vy_quota *q) { + assert(q->is_enabled); if (q->dump_in_progress) return; if (q->trigger_dump_cb(q) != 0) @@ -87,7 +88,7 @@ vy_quota_trigger_dump(struct vy_quota *q) static void vy_quota_check_watermark(struct vy_quota *q) { - if (q->used >= q->watermark) + if (q->is_enabled && q->used >= q->watermark) vy_quota_trigger_dump(q); } @@ -99,6 +100,8 @@ vy_quota_timer_cb(ev_loop *loop, ev_timer *timer, int events) struct vy_quota *q = timer->data; + assert(q->is_enabled); + /* * Update the quota use rate with the new measurement. */ @@ -125,7 +128,8 @@ vy_quota_timer_cb(ev_loop *loop, ev_timer *timer, int events) } int -vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb) +vy_quota_create(struct vy_quota *q, size_t limit, + vy_quota_trigger_dump_f trigger_dump_cb) { memset(q, 0, sizeof(*q)); @@ -149,8 +153,8 @@ vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb) return -1; } - q->limit = SIZE_MAX; - q->watermark = SIZE_MAX; + q->limit = limit; + q->watermark = limit; q->too_long_threshold = TIMEOUT_INFINITY; q->dump_bw = VY_DEFAULT_DUMP_BANDWIDTH; q->trigger_dump_cb = trigger_dump_cb; @@ -158,11 +162,20 @@ vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb) ev_timer_init(&q->timer, vy_quota_timer_cb, 0, VY_QUOTA_UPDATE_INTERVAL); q->timer.data = q; - ev_timer_start(loop(), &q->timer); return 0; } void +vy_quota_enable(struct vy_quota *q) +{ + assert(!q->is_enabled); + q->is_enabled = true; + q->use_curr = 0; + ev_timer_start(loop(), &q->timer); + vy_quota_check_watermark(q); +} + +void vy_quota_destroy(struct vy_quota *q) { ev_timer_stop(loop(), &q->timer); @@ -227,6 +240,8 @@ vy_quota_force_use(struct vy_quota *q, size_t size) static bool vy_quota_may_use(struct vy_quota *q, size_t size) { + if (!q->is_enabled) + return true; return q->used + size <= q->limit; } diff --git a/src/box/vy_quota.h b/src/box/vy_quota.h index 3b020829..793a4430 100644 --- a/src/box/vy_quota.h +++ b/src/box/vy_quota.h @@ -51,6 +51,8 @@ typedef int * in the vinyl engine. It is NOT multi-threading safe. */ struct vy_quota { + /** Set when the quota is enabled. */ + bool is_enabled; /** * Memory limit. Once hit, new transactions are * throttled until memory is reclaimed. @@ -115,9 +117,30 @@ struct vy_quota { struct histogram *dump_bw_hist; }; +/** + * Initialize a quota object. + * + * @limit: max allowed memory usage. + * @trigger_dump_cb: callback invoked to trigger memory dump. + * + * Returns 0 on success, -1 on memory allocation error. + * + * Note, the limit won't be imposed until vy_quota_enable() + * is called. + */ int -vy_quota_create(struct vy_quota *q, vy_quota_trigger_dump_f trigger_dump_cb); +vy_quota_create(struct vy_quota *q, size_t limit, + vy_quota_trigger_dump_f trigger_dump_cb); +/** + * Enable the configured limit for a quota object. + */ +void +vy_quota_enable(struct vy_quota *q); + +/** + * Destroy a quota object. + */ void vy_quota_destroy(struct vy_quota *q); -- 2.11.0
next prev 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 ` [PATCH 03/11] vinyl: do not try to trigger dump if it is already in progress Vladimir Davydov 2018-09-20 9:34 ` Vladimir Davydov [this message] 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=6f21c27a5f71c969dce68daffe4ef4d1f852e5ac.1537435404.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 04/11] vinyl: don'\''t start quota timer until local recovery is complete' \ /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