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

AtomicExpiringValue #191

Merged
merged 9 commits into from
Aug 1, 2023
Merged

AtomicExpiringValue #191

merged 9 commits into from
Aug 1, 2023

Conversation

didierofrivia
Copy link
Contributor

@didierofrivia didierofrivia commented Jul 18, 2023

This PR is one step closer to a multi-threading Limitador. It introduces the AtomicExpiringValue, as the name suggests, an atomic alternative to the ExpiringValue type, at the moment used only in InMemory storage. This wraps the counter values within an AtomicI64, which makes it possible to safely update the values with multiple threads.

A minor regression was introduced with this change, but will be solved when we get rid of the "normalization" of counters elsewhere and not in check_and_update InMemory method.

Screenshot 2023-08-01 at 15 48 37

In memory/is_rate_limited/10 namespaces with 50 limits each with 10 conditions and 10 variables

Screenshot 2023-08-01 at 15 50 40

In memory/is_rate_limited/10 namespaces with 10 limits each with 10 conditions and 10 variables

Screenshot 2023-08-01 at 15 51 14

In memory/is_rate_limited/1 namespaces with 1 limits each with 1 conditions and 1 variables

Screenshot 2023-08-01 at 15 51 35

@didierofrivia didierofrivia force-pushed the atomic-expiring-value branch 2 times, most recently from 17626cc to 53ae626 Compare July 26, 2023 11:21
// Update counters
counter_values_to_update
.iter()
.for_each(|(v, ttl)| v.update(delta, *ttl, now));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only not racy right now because of the WriteLock acquired above.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@775596d). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #191   +/-   ##
=======================================
  Coverage        ?   74.10%           
=======================================
  Files           ?       30           
  Lines           ?     4989           
  Branches        ?        0           
=======================================
  Hits            ?     3697           
  Misses          ?     1292           
  Partials        ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

* Using thread.scope in order to get rid of Arc and Barrier
* Fixing assertion to include possible accepted values
@didierofrivia didierofrivia marked this pull request as ready for review July 31, 2023 14:43
@didierofrivia didierofrivia merged commit 5377654 into main Aug 1, 2023
17 checks passed
@didierofrivia didierofrivia deleted the atomic-expiring-value branch August 1, 2023 13:52
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

Successfully merging this pull request may close these issues.

3 participants