SGrondin / bottleneck

Job scheduler and rate limiter, supports Clustering
MIT License
1.83k stars 78 forks source link

Feature: Store Group settings in Redis #106

Open weeco opened 5 years ago

weeco commented 5 years ago

I am calling UpdateSettings() right after creating in the group in order to make sure that settings changes will be applied after redeploying. Unfortunately it seems like updateSettings() doesn't have any effect as the settings stored in redis didn't change at all (I tried to change the reservoir settings):

  public async onModuleInit(): Promise<void> {
    const groupOptions: bottleneck.ConstructorOptions = {
      highWater: 0,
      id: 'auth-limit',
      strategy: bottleneck.strategy.OVERFLOW,
      reservoir: 2,
      reservoirRefreshAmount: 2,
      reservoirRefreshInterval: 15 * 1000,
      datastore: 'ioredis',
      clearDatastore: false,
      clientOptions: {
        host: this.config.redis.host,
        port: this.config.redis.port
      },
      timeout: 5 * 60 * 1000
    };
    this.group = new bottleneck.Group(groupOptions);
    await this.group.updateSettings(groupOptions);
SGrondin commented 5 years ago

That's correct. Unfortunately right now Groups themselves are not stored in Redis, only the limiters inside Groups. I agree it would be better to store them in Redis as well. What I'd like to know is how much work do you have to do to get around this limitation? I have plans to store everything in Redis, including jobs, but we're talking months of work broken down into several releases.

The more I think about this, the more I believe that this feature is a good place to start.

SGrondin commented 5 years ago

Your name sounded familiar so I ran a search over closed issues and sure enough, you're one of the earliest people who requested Cluster support! It's really come a long way since I first released it a year ago. Group settings and jobs data aren't in Redis today is because distributed systems are extremely hard and more than almost any other type of software they need to be grown, not designed. I wanted to limit the number of features to a minimum to make it easier to refactor and debug.

The Clustering code has gone through several incremental refactors in 2018 and I now believe it's mature enough to add new features to it I also don't want to break backwards compatibility unless absolutely necessary. All these things mean that a project such as Bottleneck evolves slowly compared to other types of software. The really big feature I have planned for 2019 is storing jobs in Redis to get rid of all these limitations.

Have you been using it all this time? I hope it's still working it for you!

weeco commented 5 years ago

As of bypassing the limitation, I'd first need to understand how I could reset the group settings (e.g. reservoir). I was under the impression they are actually stored in redis because I find group settings there (see: https://i.imgur.com/ixXkIMe.png).

You are right it was me who requested cluster support long time ago. It was another project where I was looking for a rate limiter. I ended up creating my own rate limiter using RabbitMQ as I needed the cluster support and job scheduling. It did the job very well.

For my current project I use a rate limiter in the common sense - purely to block Layer 7 (D)DoS attacks based on JWTs / IpAddresses. I believe you that it's incredibly hard to code reliable, robust distributed systems which are easy to use and understand at the same time. I came back to bottleneck because I believe it's the best library to do the job in the nodejs world at the moment. The only con for me is that it's not written in TypeScript yet, but luckily it had typings :D.

SGrondin commented 5 years ago

"Group settings" are basically just a template for limiter settings. When a limiter is created (by calling group.key(id);) it gets its settings from the Group and stores those in Redis. The limiter is then 100% independent from the Group. Updating the Group settings (aka the template) doesn't change the settings of existing limiters. Perhaps the docs need to make this clearer. In general there are so many caveats around updating limiter settings that I'm thinking about removing next time I need to make breaking changes. I would also like to rename group.updateSettings() to group.updateTemplate().

Anyways, for your workaround, I suggest you simply store that template somewhere. It could be as simple as a json file served from a URL. You could even manually store it in Redis. I don't want to promise anything but I think I could reasonably release this feature in late February.

And yeah eventually I'll port the code to something else, most likely TypeScript, but right now it's simply not worth the time.

SGrondin commented 5 years ago

Please let me know if you have other questions, I'm happy to help you get around this limitation until the feature is added to Bottleneck directly.

SGrondin commented 5 years ago

I'm curious, assuming Clustering and Groups are in use:

Essentially, there are 2 different ways I see to implement this feature:

Between those two, I think I prefer Option 1, but I'd like to hear your thoughts on this.


Independently of the above, I'm also thinking about how the API should look like.

All of these questions make me wonder if we're approaching the problem from the right angle. Your original problem was "How do I persist Group settings across restarts". What if what you needed was a helper tool to leverage Bottleneck's Redis tooling to easily store and retrieve data from Redis?

Option 3 We already have const connection = Bottleneck.RedisConnection({ redis options }), so what if you could do something like this:

const connection = Bottleneck.RedisConnection({ redisOptions });

const groupSettings = await connection.fetchObject('my-group-settings');

const group = new Bottleneck.Group({ connection, ...groupSettings });

and somewhere else in the code:

const connection = group.connection;

await connection.storeObject('my-group-settings', group.settings());

A lot of this example code already works as is today and the rest is already implemented internally by Bottleneck, it just needs to be exposed to the user. This Option 3 is a lot more explicit than Options 1 and 2. I think that making it explicit like that would eliminate a lot of the confusion.

Please let me know what you think!