Il 16/08/2013 13:45, Stefan Hajnoczi ha scritto:
>> > +#define BUCKETS_COUNT 6
>> > +
>> > +typedef enum {
>> > + THROTTLE_BPS_TOTAL = 0,
>> > + THROTTLE_BPS_READ = 1,
>> > + THROTTLE_BPS_WRITE = 2,
>> > + THROTTLE_OPS_TOTAL = 3,
>> > + THROTTLE_OPS_READ = 4,
>> > + THROTTLE_OPS_WRITE = 5,
>> > +} BucketType;
Please remove the "= N" from the enums, and add BUCKETS_COUNT here.
>> > +typedef struct LeakyBucket {
>> > + double ups; /* units per second */
>> > + double max; /* leaky bucket max in units */
>> > + double bucket; /* bucket in units */
> These comments aren't very clear to me :). So I guess bps or iops would
> be in ups. Max would be the total budget or maximum burst. Bucket
> might be the current level.
I also suggest replacing "ups" with "avg", since it's the average
throughput that the leaky bucket allows after the initial burst has
emptied the bucket.
>> + uint64_t unit_size; /* size of an unit in bytes */
>> + uint64_t op_size; /* size of an operation in units */
>
> It's not clear yet why we need both unit_size *and* op_size. I thought
> you would have a single granularity field for accounting big requests as
> multiple iops.
IIUC the ops buckets account operations in op_size / unit_size units,
while the bps buckets account operations in 1 / unit_size units, or
something like that. But it needs clarification indeed.
Paolo
