Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token bucket once again #47

Closed

Conversation

Labels
None yet
Projects
None yet
2 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Apr 13, 2018

No description provided.

nmathewson added 6 commits Apr 13, 2018
This is a simple search-and-replace to rename the token bucket type
to indicate that it contains both a read and a write bucket, bundled
with their configuration.  It's preliminary to refactoring the
bucket type.
Begin by creating a lowest-level triple of the types needed to
implement a token bucket: a configuration, a timestamp, and the raw
bucket itself.

Note that for low-level buckets, the units of the timestamp and the
bucket itself are unspecified: each user can use a different type.

(This patch breaks check-spaces; a later patch will fix it)
(The tests caught this one.)
(These functions were previously helper functions for
token_bucket_rw_t).
Really, the uint32_t is only an optimization; any kind of unit
should work fine.  Some users might want to use time_t or
monotime_coarse_t or something like that.
/** A timestamp for a token bucket. The units of this timestamp are
* unspecified, but must match with the rate set in the token_bucket_cfg_t. */
typedef struct token_bucket_timestamp_t {
uint32_t last_refilled_at;
Copy link
Contributor

@dgoulet-tor dgoulet-tor Apr 13, 2018

If this is a Unix timestamp, this means we are stuck in 2038 ... :S Could we go to uint64_t to avoid that?

@@ -20,6 +21,51 @@
#include "token_bucket.h"
#include "util_bug.h"

/**
*Set the <b>rate</b> and <b>burst</b> value in a token_bucket_cfg.
*
Copy link
Contributor

@dgoulet-tor dgoulet-tor Apr 13, 2018

Whitespace missing.

Copy link
Contributor Author

@nmathewson nmathewson Apr 13, 2018

Fixed in 1be0247

token_bucket_raw_reset(token_bucket_raw_t *bucket,
token_bucket_timestamp_t *stamp,
const token_bucket_cfg_t *cfg,
uint32_t now)
Copy link
Contributor

@dgoulet-tor dgoulet-tor Apr 13, 2018

Again, if now is a Unix timestamp, we are a bit stuck in 2038.

* rate in <b>cfg</b>, or the result will be illogical.
*/
int
token_bucket_raw_refill_steps(token_bucket_raw_t *bucket,
Copy link
Contributor

@dgoulet-tor dgoulet-tor Apr 13, 2018

This can be made static.

Copy link
Contributor Author

@nmathewson nmathewson Apr 13, 2018

No; it should be public, so that others can use token_bucket_raw.

Copy link
Contributor Author

@nmathewson nmathewson Apr 13, 2018

That is to say, the idea of the refactoring here is that the token_bucket_raw() functions can be used by outside code to build token buckets that aren't specifically tied to token_bucket_rw_t.

@nmathewson
Copy link
Contributor Author

@nmathewson nmathewson commented May 7, 2018

This was squashed and merged with 34c2574

@nmathewson nmathewson closed this May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment