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

TTL support for DynamoDB backend #265

Closed
Avinm opened this issue May 31, 2022 · 8 comments
Closed

TTL support for DynamoDB backend #265

Avinm opened this issue May 31, 2022 · 8 comments

Comments

@Avinm
Copy link

Avinm commented May 31, 2022

I noticed that the redis implementation has an option to add TTL and dynamo one does not.
Was this ever considered?
Could try raising a PR if there's nothing blocking the implementation.

@vladimir-bukhtoyarov
Copy link
Collaborator

Hello @Avinm

If dynamoDb has TTL feature, there are nothing that prevents to add TTL to DynamoDb integration. Feel free to prepare pull request if you have a time to do this.

@Avinm
Copy link
Author

Avinm commented Jun 2, 2022

Yes, DynamoDB does have a TTL feature: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/TTL.html
However, items are not immediately deleted. Therefore it is upon the implementation to filter out older items that are pending deletion.
Do you have any recommendation on how to decide the TTL window? I'm thinking that ideally the TTL window is computed by bucket4j as the largest time window among all rate limiting rules, but it looks like the redis implementation takes in the TTL parameter as input?

@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Jun 3, 2022

Currently, I recommend to choose TTL by following criteria:

  1. TTL should be >= max proposed time window among all rate limiting rules.
  2. Also if capacity > refill_tokens, it is possible where classic bandwidth is used, then TTL choosed at previous step should be multiplied by X, where X = capacity/refill_tokens.

However, I want to notice that current support of TTL for redis is awkward and choosing right TTL is hard, and even impossible sometimes. There is better implementation of TTL in the CaffeineProxyManager that does not require to provide initial estimates, instead it relies on the time that required to full refilling of consumed tokens, keepAfterRefillDuration is not critical and can be zero.

Please wait for Monday, I will add this concept to the AbstractCompareAndSwapBasedProxyManager and you will be able to write the code similar that written in CaffeineProxyManager.

vladimir-bukhtoyarov added a commit that referenced this issue Jun 4, 2022
…mpareAndSwapOperation, SelectForUpdateBasedTransaction and LockBasedTransaction. In order to provide ability for accurate TTL calculation.
@vladimir-bukhtoyarov
Copy link
Collaborator

@Avinm the branch 7.6 is the right branch to start your pull request. RedissonBasedProxyManager has been modified in this branch, use it as example of flexible TTL calculation.

@augustlakia
Copy link

Is this issue still relevant? @Avinm are you working on this ?

@vladimir-bukhtoyarov
Copy link
Collaborator

Hello @augustlakia

The issue is actual. It should be easy exercise, but I have not time currently to accomplish it, so you are welcome to provide pull-request.

@Avinm
Copy link
Author

Avinm commented Sep 14, 2022

@augustlakia
Unfortunately I haven't yet gotten around to starting on this. This is because I realized that I would currently only be using this on finite cardinality keys.
Feel free to raise a PR if you want to pick it up, if not, I'll give a heads up here a couple of days before I start

@vladimir-bukhtoyarov
Copy link
Collaborator

Declined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants