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

Push API should support multiple notifications in a single push (bulk API) #609

Open
Half-Shot opened this issue Apr 15, 2020 · 3 comments
Labels
A-Push feature Suggestion for a significant extension which needs considerable consideration

Comments

@Half-Shot
Copy link
Contributor

It currently supports one notification but there is some evidence to suggest it would be more efficient to deliver these in bulk transaction-like requests.

@turt2live turt2live added feature Suggestion for a significant extension which needs considerable consideration A-Push labels Apr 15, 2020
@richvdh
Copy link
Member

richvdh commented Jul 14, 2020

This has interactions with how we manage reliability of push when sygnal is unreachable, or restarted while processing a push request:

Currently synapse makes no attempt to resend pushes which don't get a successful response from sygnal; however that obviously leads to unreliability. Assuming we wanted to fix that, we might need to give pushes transaction ids so that sygnal can de-duplicate. We should consider how we would extend that to transactions which contain multiple pushes. (Although a simple mechanism would be just to give each push within a transaction its own ID.)

@reivilibre
Copy link
Contributor

Each push in a transaction can be identified by (transaction ID, index within the transaction).

I do wonder if this would mean making Sygnal stateful (currently it is virtually stateless).

Also a great chance to fix the issue of not being able to indicate errors with per-push granularity.

@richvdh
Copy link
Member

richvdh commented Feb 15, 2022

It currently supports one notification

Technically speaking, it supports sending the same notification to multiple devices at a time, via the devices parameter. That said, it's not currently used. It's of vague interest for MSC3013 which would make it totally useless.

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Push feature Suggestion for a significant extension which needs considerable consideration
Projects
None yet
Development

No branches or pull requests

4 participants