Skip to content

Add SAFERATE spec #556

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add SAFERATE spec #556

wants to merge 1 commit into from

Conversation

delthas
Copy link
Contributor

@delthas delthas commented Oct 26, 2024

This is an "implementation" of the rate limiting ircv3-idea, for the first possible solution described in it: "Add a new cap to indicate that a server won't disconnect a client because of rate limits. The server would still be free to slowly process message bursts, this would result in writes blocking on the client-side.".

Implementations

  • Bouncer: soju (as client and server) (implemented as soju.im/SAFERATE)
  • Server: InspIRCd (implemented as SAFERATE)
  • Server: ergo (implemented as SAFERATE)

cc @slingamn @emersion

@SadieCat
Copy link
Contributor

I'm not sure this is usable in practice. Even with fakelag users will eventually hit some kind of hard limit on most implementations.

@slingamn
Copy link
Contributor

Just as an example: current versions of Ergo in the default/recommended configuration could publish this token. Writes will start to block on the client side (the limiting factor in practice is the TCP buffer size) but there is no "hard limit".

@slingamn
Copy link
Contributor

re. usefulness to implementations, I believe it is demonstrably useful to soju and senpai?

@SadieCat
Copy link
Contributor

I'm not sure another specification that is only usable by Ergo is a good idea. Maybe this should be a vendor extension?

@slingamn
Copy link
Contributor

This specification is implemented by Soju-as-a-server as well, but I didn't comment on the implementation details there since I'm not familiar with them.

@delthas
Copy link
Contributor Author

delthas commented Mar 6, 2025

Edit: Added InspIRCd support.

slingamn added a commit to slingamn/ergo that referenced this pull request Mar 6, 2025
@delthas
Copy link
Contributor Author

delthas commented Mar 10, 2025

Edit: Added ergo support.

@emersion
Copy link
Contributor

Now that we have 3 implementations, can we merge this @jwheare?

@slingamn
Copy link
Contributor

Someone in #ircv3 was arguing that in principle, the protocol requires every client to maintain two queues, a low-priority queue for ordinary messages and a high-priority queue for sending PONG in a timely fashion. I don't think most implementations worry about this, but it might be worth a note in the "implementation considerations" section that a server publishing this token should relax its expectations around PONG, i.e. tolerate PONG being delayed because it's stuck in the TCP buffer behind fakelag.

@DarthGandalf
Copy link
Member

the protocol requires every client to maintain two queues

ZNC currently has a single queue, but PONG gets pushed to the front of the queue instead of back

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.

5 participants