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

Prevent proxy lock when used by different pads #17

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

danielpetri1
Copy link
Contributor

@danielpetri1 danielpetri1 commented Nov 1, 2022

Currently, usage of the export proxy locks Etherpad's API entirely until the request is handled.

At scale, this can becomes problematic under these circumstances:

  • multiple recordings processing at the same time may not be able to download the shared notes
  • in 2.6, shared notes can automatically be sent back as presentations from breakout rooms once they end

By including the padId in the author name, multiple requests from the same pad are still prevented, while allowing other pads from requesting the file.

With this approach, I was able to get 10 simultaneous requests exporting at the same time before getting an HTTP 429 Too Many Requests error back with an according Retry-After payload.

Prevents the API from locking when multiple pads are using the proxy.
@pedrobmarin
Copy link
Collaborator

I guess you could even use a timestamp for the author name in this case. The feature you described looks something new and when this proxy came along we probably did not have such scenario. The author name is just a requirement for the API to request a session to access the pad content. Nothing security related or anything since you can only call for this endpoint internally.

@danielpetri1
Copy link
Contributor Author

Perfect, thanks for reviewing this so quickly.
Concerning the limit of 10 I was hitting before, this setting can be changed in Etherpad's importExportRateLimiting.max:

  "importExportRateLimiting": {
    // duration of the rate limit window (milliseconds)
    "windowMs": 90000,

    // maximum number of requests per IP to allow during the rate limit window
    "max": 16
  },

I am setting this to 16 in PR bigbluebutton/bigbluebutton#15868 since that is the default maximum number of breakout rooms.

@prlanzarin
Copy link
Member

cc @danielpetri1 @pedrobmarin I'll tag this soon and bump the placeholder in v2.6.x-release afterwards.

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