From: Konstantin Osipov <kostja@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH v2 11/11] vinyl: introduce quota consumer priorities Date: Sat, 6 Oct 2018 16:24:05 +0300 [thread overview] Message-ID: <20181006132405.GD1380@chai> (raw) In-Reply-To: <a6e72e69c3ed96b3ff5869beac98172b4bb80803.1538155645.git.vdavydov.dev@gmail.com> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/09/28 21:00]: Since we agreed to go with the implemented approach, please see a few implementation comments below. > Currently, we only limit quota consumption rate so that writers won't > hit the hard limit before memory dump is complete. However, it isn't > enough, because we also need to consider compaction: if it doesn't keep > up with dumps, read and space amplification will grow uncontrollably. > > The problem is compaction may be a quota consumer by itself, as it may > generate deferred DELETE statements for secondary indexes. We can't > ignore quota completely there, because if we do, we may hit the memory > limit and stall all writers, which is unacceptable, but we do want to > ignore the rate limit imposed to make sure that compaction keeps up with > dumps, otherwise compaction won't benefit from such a throttling. > > To tackle this problem, this patch introduces quota consumer priorities. > Now a quota object maintains one rate limit and one wait queue per each > transaction priority. Rate limit i is used by a consumer with priority > prio if and only if prio <= i. Whenever a consumer has to be throttled, > it is put to sleep to the wait queue corresponding to its priority. > When quota is replenished, we pick the oldest consumer among all that > may be woken up. This ensures fairness. As discusses, these are not priorities, these are resource types. Please change the patch to pass a bit mask of resource types for which we request the quota. Quota refill interval should be varying - please schedule this work in a separate patch. > static void > vy_quota_signal(struct vy_quota *q) > { > - if (!rlist_empty(&q->wait_queue)) { > + /* > + * Wake up a consumer that has waited most no matter > + * whether it's high or low priority. This assures that > + * high priority consumers don't uncontrollably throttle > + * low priority ones. > + */ > + struct vy_quota_wait_node *oldest = NULL; > + for (int i = 0; i < vy_quota_consumer_prio_MAX; i++) { > + struct rlist *wq = &q->wait_queue[i]; > + if (rlist_empty(wq)) > + continue; I still do not understand why you need a second queue and timestapms. If you need to ensure total fairness, a single queue should be enough. > + > struct vy_quota_wait_node *n; > - n = rlist_first_entry(&q->wait_queue, > - struct vy_quota_wait_node, in_wait_queue); > + n = rlist_first_entry(wq, struct vy_quota_wait_node, > + in_wait_queue); > /* > * No need in waking up a consumer if it will have > * to go back to sleep immediately. > */ > - if (vy_quota_may_use(q, n->size)) > - fiber_wakeup(n->fiber); > + if (!vy_quota_may_use(q, i, n->size)) > + continue; > + > + if (oldest == NULL || oldest->timestamp > n->timestamp) > + oldest = n; > } > + if (oldest != NULL) > + fiber_wakeup(oldest->fiber); This is some kind of black magic to me. Imagine you have another resource type, say, CPU? Will you add the third queue? How this will piece of code look then? I'm OK if you go with a single queue in which you register all requests, regardless of whether they are for disk, memory, or both - and satisfy all requests in vy_quota_signal(). But I don't immediately understand the magic with two wait queues - and frankly still don't see any need for it. > + /** > + * Timestamp assigned to this fiber when it was put to > + * sleep, see vy_quota::wait_timestamp for more details. > + */ > + int64_t timestamp; If you have a single queue, you don't need this. The name is to general in any case. > }; > > /** > @@ -144,13 +186,27 @@ struct vy_quota { > */ > vy_quota_exceeded_f quota_exceeded_cb; > /** > - * Queue of consumers waiting for quota, linked by > - * vy_quota_wait_node::state. Newcomers are added > - * to the tail. > + * Monotonically growing timestamp assigned to consumers > + * waiting for quota. It is used for balancing wakeups > + * among wait queues: if two fibers from different wait > + * queues may proceed, the one with the lowest timestamp > + * will be picked. > + * > + * See also vy_quota_wait_node::timestamp. > + */ > + int64_t wait_timestamp; -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov
next prev parent reply other threads:[~2018-10-06 13:24 UTC|newest] Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-28 17:39 [PATCH v2 00/11] vinyl: transaction throttling infrastructure Vladimir Davydov 2018-09-28 17:39 ` [PATCH v2 01/11] vinyl: add helper to start scheduler and enable quota on startup Vladimir Davydov 2018-09-29 4:37 ` [tarantool-patches] " Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 02/11] vinyl: factor load regulator out of quota Vladimir Davydov 2018-09-29 5:00 ` [tarantool-patches] " Konstantin Osipov 2018-09-29 11:36 ` Vladimir Davydov [not found] ` <20180929114308.GA19162@chai> 2018-10-01 10:27 ` Vladimir Davydov 2018-10-01 10:31 ` Vladimir Davydov 2018-10-02 18:16 ` [tarantool-patches] " Konstantin Osipov 2018-10-03 8:49 ` Vladimir Davydov 2018-09-28 17:40 ` [PATCH v2 03/11] vinyl: minor refactoring of quota methods Vladimir Davydov 2018-09-29 5:01 ` [tarantool-patches] " Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 04/11] vinyl: move transaction size sanity check to quota Vladimir Davydov 2018-09-29 5:02 ` [tarantool-patches] " Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 05/11] vinyl: implement quota wait queue without fiber_cond Vladimir Davydov 2018-09-29 5:05 ` [tarantool-patches] " Konstantin Osipov 2018-09-29 11:44 ` Vladimir Davydov 2018-09-28 17:40 ` [PATCH v2 06/11] vinyl: enable quota upon recovery completion explicitly Vladimir Davydov 2018-09-29 5:06 ` [tarantool-patches] " Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 07/11] vinyl: zap vy_env::memory, read_threads, and write_threads Vladimir Davydov 2018-09-29 5:06 ` [tarantool-patches] " Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 08/11] vinyl: do not try to trigger dump in regulator if already in progress Vladimir Davydov 2018-09-28 17:40 ` [PATCH v2 09/11] vinyl: do not account zero dump bandwidth Vladimir Davydov 2018-10-12 13:27 ` Vladimir Davydov 2018-10-16 18:25 ` [tarantool-patches] " Konstantin Osipov 2018-10-17 8:44 ` Vladimir Davydov 2018-10-23 7:02 ` Konstantin Osipov 2018-09-28 17:40 ` [PATCH v2 10/11] vinyl: implement basic transaction throttling Vladimir Davydov 2018-09-28 17:40 ` [PATCH v2 11/11] vinyl: introduce quota consumer priorities Vladimir Davydov 2018-10-06 13:24 ` Konstantin Osipov [this message] 2018-10-08 11:10 ` Vladimir Davydov 2018-10-09 13:25 ` Vladimir Davydov 2018-10-11 7:02 ` Konstantin Osipov 2018-10-11 8:29 ` Vladimir Davydov 2018-10-03 9:06 ` [PATCH v2 00/11] vinyl: transaction throttling infrastructure Vladimir Davydov
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=20181006132405.GD1380@chai \ --to=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH v2 11/11] vinyl: introduce quota consumer priorities' \ /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