Fix handling of group rate limits under 64 bytes of burst

The "min_share" logic, which was designed to prevent piles of
extremely small writes when running up against a group rate limit,
could lead to confusing behavior if you ever set a min_share less
than your burst rate.  If that happened, then as soon as your group
rate limit was exhausted, you'd stop reading/writing, and never
start again, since the amount readable/writeable would never
actually hit min_share.

We now cap min_share at the rate per tick.

Found by George Kadianakis
This commit is contained in:
Nick Mathewson 2011-08-11 15:15:17 -04:00
parent 5d1b255b14
commit 6d5440e80e
2 changed files with 17 additions and 1 deletions

View File

@ -98,6 +98,8 @@ struct bufferevent_rate_limit_group {
/** The smallest number of bytes that any member of the group should /** The smallest number of bytes that any member of the group should
* be limited to read or write at a time. */ * be limited to read or write at a time. */
ev_ssize_t min_share; ev_ssize_t min_share;
ev_ssize_t configured_min_share;
/** Timeout event that goes off once a tick, when the bucket is ready /** Timeout event that goes off once a tick, when the bucket is ready
* to refill. */ * to refill. */
struct event master_refill_event; struct event master_refill_event;

View File

@ -636,13 +636,15 @@ bufferevent_rate_limit_group_new(struct event_base *base,
ev_token_bucket_init(&g->rate_limit, cfg, tick, 0); ev_token_bucket_init(&g->rate_limit, cfg, tick, 0);
g->min_share = 64;
event_assign(&g->master_refill_event, base, -1, EV_PERSIST, event_assign(&g->master_refill_event, base, -1, EV_PERSIST,
_bev_group_refill_callback, g); _bev_group_refill_callback, g);
/*XXXX handle event_add failure */ /*XXXX handle event_add failure */
event_add(&g->master_refill_event, &cfg->tick_timeout); event_add(&g->master_refill_event, &cfg->tick_timeout);
EVTHREAD_ALLOC_LOCK(g->lock, EVTHREAD_LOCKTYPE_RECURSIVE); EVTHREAD_ALLOC_LOCK(g->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
bufferevent_rate_limit_group_set_min_share(g, 64);
return g; return g;
} }
@ -670,6 +672,9 @@ bufferevent_rate_limit_group_set_cfg(
event_add(&g->master_refill_event, &cfg->tick_timeout); event_add(&g->master_refill_event, &cfg->tick_timeout);
} }
/* The new limits might force us to adjust min_share differently. */
bufferevent_rate_limit_group_set_min_share(g, g->configured_min_share);
UNLOCK_GROUP(g); UNLOCK_GROUP(g);
return 0; return 0;
} }
@ -682,6 +687,15 @@ bufferevent_rate_limit_group_set_min_share(
if (share > EV_SSIZE_MAX) if (share > EV_SSIZE_MAX)
return -1; return -1;
g->configured_min_share = share;
/* Can't set share to less than the one-tick maximum. IOW, at steady
* state, at least one connection can go per tick. */
if (share > g->rate_limit_cfg.read_rate)
share = g->rate_limit_cfg.read_rate;
if (share > g->rate_limit_cfg.write_rate)
share = g->rate_limit_cfg.write_rate;
g->min_share = share; g->min_share = share;
return 0; return 0;
} }