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.

Sign up for GitHub

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

Jump to bottom

[performance] remove throttling timers #1466

Merged
merged 7 commits into from
Feb 10, 2023
Merged

[performance] remove throttling timers #1466

merged 7 commits into from
Feb 10, 2023

Conversation

NyaaaWhatsUpDoc
Copy link
Member

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc commented Feb 9, 2023 •
edited
Loading

We already set request read/write timeouts (which will in turn cancel the request contexts). Timers in the throttling middleware are un-needed, and under load will add a lot of running timers. This also adds:

  • support custom setting for throttling retry-after
  • use retry-after header in transport controller

Checklist:

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@NyaaaWhatsUpDoc
remove throttling timers, support setting retry-after, use retry-afte…
1b7b1e7 
…r in transport

Signed-off-by: kim <grufwub@gmail.com>
@NyaaaWhatsUpDoc
Copy link
Member Author

Just need to update example configuration and the CLI/env parsing tests and will be ready.

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc marked this pull request as ready for review February 9, 2023 19:28
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc merged commit 70739d3 into superseriousbusiness:main Feb 10, 2023
firescry added a commit to firescry/gotosocial that referenced this pull request Jul 9, 2023
@firescry
Add default value for AdvancedThrottlingRetryAfter to internal/config…
0a39b70 
…/defaults.go

AdvancedThrottlingRetryAfter was introduced in 70739d3 (superseriousbusiness#1466).
@firescry firescry mentioned this pull request Jul 9, 2023
Merged
9 tasks
firescry added a commit to firescry/gotosocial that referenced this pull request Jul 11, 2023
@firescry
Add default value for AdvancedThrottlingRetryAfter to internal/config…
69294f3 
…/defaults.go

AdvancedThrottlingRetryAfter was introduced in 70739d3 (superseriousbusiness#1466).
tsmethurst pushed a commit that referenced this pull request Jul 12, 2023
@firescry
[bugfix] Align default values in the configuration file with the code (…
8d92b24 
…#1971)

* Set default value of SMTPFrom to empty string

This parameter should contain proper e-mail address (to be provided by user during configuration).

* Update default values in example/config.yaml

Default values and related comments in example/config.yaml are aligned
with values defined in internal/config/defaults.go.
Small improvements to foramting of config.yaml file.

* Add default value for AdvancedThrottlingRetryAfter to internal/config/defaults.go

AdvancedThrottlingRetryAfter was introduced in 70739d3 (#1466).

* Update config.yaml snippets in documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsmethurst tsmethurst tsmethurst left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NyaaaWhatsUpDoc @tsmethurst