Throttle only supports synchronous datastores

From the API Guide:

In some circumstances, you want to offload this into a shared system, such as Redis, if you have a fleet of API servers and you're not getting steady and/or uniform request distribution. To enable this, you can pass in options.tokensTable, which is simply any Object that supports put and get with a String key, and an Object value.

I went to create a Redis tokensTable and quickly noticed that it won't actually work because for Redis the tokensTable.get needs to be asynchronous. The way that TokenBucket works also expects the same bucket instance to be used.

So I went and made backwards-compatible modifications to tokens.js that allow you to use either synchronous or asynchronus TokensTable implementations, which only changes the api for async ones such that it doesn't break any existing versions.

I've got it nearly there, but the more I work on it the more I think a larger-order refactor of the throttle.js would make things a lot easier (or at least cleaner). I don't mind doing this, but I'm thinking if I did, it may be a reasonable time to extract it from restify into its own module.

So my questions are:

First: Am I missing something here that would make it feasible to use an async TokensTable with the current implementation and wasting my time?

Second: If I make a new external throttle module that supports both the backwards-compatible synchronous TokensTable and asynchronous versions, would you accept a pull request that removes the existing implementation for a dependency on the new one?

Hi,

No, it probably needs to be refactored to work with redis or something. And yes, I would happily offload this; I'd probably leave what's there in, and just point people at the external one.

@brycebaril

@brycebaril did you manage to pull this off?

@gergelyke

@gergelyke Yes, to an extent -- I made https://npmjs.org/package/tokenthrottle-redis which you should be able to plug in similarly to restify. It is heavily based on @mcavage 's implementation.

I haven't gone through the effort of writing up how to plug it into restify, however.

Could you do that please?

@gergelyke

Having looked briefly at tokenthrottle I'd definitely want to merge this
functionality back into restify (having an async way to get a throttle
decision) - the redis one I don't really want to ship with (restify is
heavy enough w/o adding another big dep on redis), but we can certainly get
this unified so there's a friendly way for others to do so.

On Fri, Feb 7, 2014 at 8:19 AM, Bryce Baril notifications@github.comwrote:

@gergelyke https://github.com/gergelyke Yes, to an extent -- I made
https://npmjs.org/package/tokenthrottle-redis which you should be able to
plug in similarly to restify. It is heavily based on @mcavagehttps://github.com/mcavage's implementation.

I haven't gone through the effort of writing up how to plug it into
restify, however.

Reply to this email directly or view it on GitHubhttps://github.com//issues/381#issuecomment-34460386
.

@brycebaril

Just finished up a project where we needed a redis backed store for our throttling. Utilized https://npmjs.org/package/tokenthrottle-redis by @brycebaril

Here's a quick snippet of the pattern I used:

// Create restify server
var restify = require('restify');
var server = restify.createServer({name:'my-api'});

// Create a redis client
var redisClient = require("redis").createClient()

// Create a throttle with 100 access limit per second.
var throttle = require("tokenthrottle-redis")({rate: 100, expiry: 86400}, redisClient);

// Add a simple username based throttling middleware
server.use(function(req, res, next) {
  var key = req.username;
  if (!key) return next();

  throttle.rateLimit(key, function(err, limited) {
    if (err) return next(err);

    req.log.trace('Throttle(%s): n', limitKey);

    if (limited) {
      return next(new Error("Rate limit exceeded, please slow down."));
    }
    else {
      return next()
    }
  });
});

If you need something more configurable, you could turn this into a configurable middleware, similar to the throttle plugin that ships with restify. But for my purposes, only needed username throttling and no overrides for example.

This is the same as #518, will close #518 in favor of this one.

Create FEATURE_REQUESTS.md

I was looking for a more configurable rate limit module
We are using memcached instead au redis

this nest plugin does axactly what we want
https://github.com/nestjs/throttler

Navigation Menu

Repository navigation

Throttle only supports synchronous datastores #381

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Throttle only supports synchronous datastores #381

Description

Activity

mcavage commented on Jul 19, 2013

gergelyke commented on Feb 7, 2014

brycebaril commented on Feb 7, 2014

gergelyke commented on Feb 7, 2014

mcavage commented on Feb 7, 2014

ishbu commented on Jan 23, 2015

micahr commented on Jun 18, 2015

AMorgaut commented on Jan 16, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions