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

limit rate of webxdc updates #3417

Merged
merged 13 commits into from
Jun 26, 2022
Merged

limit rate of webxdc updates #3417

merged 13 commits into from
Jun 26, 2022

Conversation

r10s
Copy link
Member

@r10s r10s commented Jun 10, 2022

this pr is based based on the ratelimited introduced by #3402:

  • instead of sending the status-update-message directly, we now just interrupt smtp; the real status-update-message is then created in the smtp-loop (similar to sync-messages and mdn) and interrupting smtp.

  • ratelimit is then handled in the same, already implemented way, resulting in smtp being interrupted again if there is another chance that something can be sent.

  • as a nice side-effect, also sync-messages are delayed as well when ratelimit happens.

closes #3311

@r10s r10s requested a review from link2xt June 10, 2022 20:24
@r10s r10s changed the base branch from master to link2xt/ratelimit June 10, 2022 20:25
src/webxdc.rs Outdated Show resolved Hide resolved
@r10s r10s mentioned this pull request Jun 10, 2022
@link2xt link2xt force-pushed the link2xt/ratelimit branch 2 times, most recently from ae8e6a9 to c0a17df Compare June 11, 2022 19:47
Base automatically changed from link2xt/ratelimit to master June 11, 2022 19:56
@r10s r10s force-pushed the webxdc-ratelimit branch 6 times, most recently from fe87f76 to 66e59b2 Compare June 12, 2022 13:45
@r10s r10s added the webxdc label Jun 13, 2022
@r10s r10s force-pushed the webxdc-ratelimit branch 7 times, most recently from aae4787 to 825ceba Compare June 13, 2022 22:13
src/webxdc.rs Show resolved Hide resolved
@r10s r10s force-pushed the webxdc-ratelimit branch 3 times, most recently from 804b638 to 75ff886 Compare June 13, 2022 22:36
@r10s r10s marked this pull request as ready for review June 14, 2022 07:42
@r10s r10s requested a review from link2xt June 14, 2022 08:08
src/webxdc.rs Outdated Show resolved Hide resolved
src/webxdc.rs Outdated Show resolved Hide resolved
src/smtp.rs Show resolved Hide resolved
src/sql/migrations.rs Outdated Show resolved Hide resolved
src/sql/migrations.rs Outdated Show resolved Hide resolved
src/webxdc.rs Outdated Show resolved Hide resolved
src/smtp.rs Outdated Show resolved Hide resolved
src/webxdc.rs Outdated Show resolved Hide resolved
@r10s
Copy link
Member Author

r10s commented Jun 18, 2022

@link2xt thanks a lot for the detailed review, i incorporated lots of your ideas. especially the sql-part is much easier now :)

for the ratelimit in send_smtp_messages(): i played around with the idea of letting webxdc-updates- and sync-messages bypass the normal sending queue. however, that seems to be quite some work and would probably double some code code and/or needs lots of refactoring.

i think the risk in adding new issues there is much larger than the effect - most times, there is only one status-update/sync anyway - and even if there are multiple, it is better than before in all cases.

so, to me, it seems that the current approach in send_smtp_messages() is "good enough", however, i added some comments, another test and moved the ratelimit check out of loops.

@r10s r10s requested a review from link2xt June 18, 2022 10:17
@link2xt link2xt mentioned this pull request Jun 18, 2022
@r10s r10s force-pushed the webxdc-ratelimit branch 2 times, most recently from bb99324 to 77eae92 Compare June 24, 2022 09:05
@r10s r10s enabled auto-merge (squash) June 26, 2022 20:03
@r10s r10s disabled auto-merge June 26, 2022 20:03
@r10s r10s merged commit 84cabbc into master Jun 26, 2022
@r10s r10s deleted the webxdc-ratelimit branch June 26, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

limit amount of updates a webxdc can send per timespan
2 participants