On Thu, Oct 11, 2018 at 10:02:50AM +0300, Konstantin Osipov wrote:
* Vladimir Davydov <vdavydov.dev@xxxxxxxxx> [18/10/08 14:12]:
Actually, there are resource types and there are consumer types.
I admit the fact that I mixed them may look confusing at the first
glance. We may introduce a seprate enum for resource types with a
mapping between them.
enum vy_quota_consumer_type {
VY_QUOTA_CONSUMER_TX = 0,
VY_QUOTA_CONSUMER_COMPACTION = 1,
vy_quota_consumer_max,
};
enum vy_quota_resource_type {
VY_QUOTA_RESOURCE_DISK = 0,
VY_QUOTA_RESOURCE_MEMORY = 1,
vy_quota_resource_max,
};
static unsigned vy_quota_consumer_mask[] = {
[VY_QUOTA_CONSUMER_TX] = (1 << VY_QUOTA_RESOURCE_DISK) |
(1 << VY_QUOTA_RESOURCE_MEMORY),
[VY_QUOTA_CONSUMER_COMPACTION] = (1 << VY_QUOTA_RESOURCE_MEMORY),
};
struct vy_quota {
...
struct rlist wait_queue[vy_quota_consumer_max];
struct vy_rate_limit rate_limit[vy_quota_resource_max];
};
This would make the code more bulky though. Do we really want to
complicate?
I like it.
Also, quite frankly I don't quite dig the concept of 'resources' and
the corresponding constant names, because 'memory' rate limit may be
confused with memory usage, which is what vy_quota is about in the first
place.
Yes, vy_quota is originally about usage. I don't know why you
insist on having rate limits in it ;) I'm ok if we put rate limits
into a separate object, now that we have vy_regulator it can
manage all of them.
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.
I need to maintain one queue per consumer type, because it's possible
that we may wake up consumers of one type, but not of the other.
If you do this, your fairness is strictly speaking broken, unless,
then again, a type implies some kind of priority. By waking up a
consumer out of (strict) order you give away a resource which may
be needed for another consumer which (in strict single-queue
order) is ahead of it.
Frankly I still don't see why you did it this way except for the
personal preference.
If we used a single queue, it could occur that consumers that
could be woken up landed in the middle of the queue so that
without scanning the queue we wouldn't be able to find them.
Scanning a queue looks ugly.
Don't scan the queue. If you have no resources for the first
consumer in the queue, everyone else has to wait. Make it simple
and stupid.
Think of the multi-queue design like this: there's one queue per each
consumer type. When a resource is replenished, we check only those
queues that store consumers that may proceed and choose the one that has
waited most.
How is that possible that some consumer can not proceed? The
replenishment is instantaneous for all kinds of resources. So if
there is at all a chance that the first (strictly first) consumer
has insufficient resources to proceed, it means it requires more
than is at all possible within this rate limit interval. It's
better to prohibit this altogether.
