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

Throttle api #184

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Throttle api #184

wants to merge 13 commits into from

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Jun 2, 2024

Making the API for throttle more friendly to use

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 23 lines in your changes missing coverage. Please review.

Project coverage is 75.65%. Comparing base (360c3a3) to head (d1efeaa).

Files Patch % Lines
src/throttle/amoc_throttle_controller.erl 91.81% 14 Missing ⚠️
src/throttle/amoc_throttle_process.erl 65.21% 8 Missing ⚠️
src/throttle/amoc_throttle_pool.erl 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   74.78%   75.65%   +0.86%     
==========================================
  Files          31       31              
  Lines        1170     1224      +54     
==========================================
+ Hits          875      926      +51     
- Misses        295      298       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NelsonVides NelsonVides force-pushed the throttle_api branch 4 times, most recently from 176d6b9 to 5f883d9 Compare June 6, 2024 14:51
@NelsonVides NelsonVides force-pushed the throttle_api branch 10 times, most recently from 572f130 to cbe472d Compare June 14, 2024 09:27
@NelsonVides NelsonVides changed the base branch from master to cosmetics June 14, 2024 09:27
@NelsonVides NelsonVides changed the title Throttle api and interarrival as throttle Throttle api Jun 14, 2024
@NelsonVides NelsonVides force-pushed the throttle_api branch 3 times, most recently from 6fdad6c to 84bfb08 Compare June 14, 2024 20:39
Base automatically changed from cosmetics to master June 17, 2024 11:45
@NelsonVides NelsonVides marked this pull request as ready for review June 30, 2024 05:12
step_size => pos_integer(),
step_count => pos_integer(),
duration => pos_integer()} |
#{from_interarrival := interarrival(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems that from_interarrival cannot be 0 nor infinity in this case, the same is applicable to to_interarrival

interval => interval(),
parallelism => non_neg_integer()}
| #{interarrival := interarrival(),
parallelism => non_neg_integer()}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the meaning of 0

%% @doc Sends and receives the given message `Msg'.
%%
%% Deprecated in favour of `wait/1'
-spec send_and_wait(name(), any()) -> ok | {error, any()}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that this interface is actively used by amoc_arsenal_xmpp. if it's so, please add tickets for amoc_arsena/amoc_arsenal_xmpp

-spec change_rate(name(), rate(), interval()) -> ok | {error, any()}.
change_rate(Name, Rate, Interval) ->
-spec change_rate(name(), throttle()) -> ok | {error, any()}.
change_rate(Name, #{rate := Rate, interval := Interval}) ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't we want to support #{interarrival := interarrival()} format here?

Amoc throttle allows to:

- Setting the execution `Rate` per `Interval`
- Limiting the number of parallel executions when `Rate` is set to `infinity`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rate is supposed to be the number of parrallel executions if Interval is 0


- Setting the execution `Rate` per `Interval`
- Limiting the number of parallel executions when `Rate` is set to `infinity`
- Setting the `Interarrival` time between actions.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not different to execution rate, but just another form of setting it.

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