Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Do not echo/send events created by appservice back out to appservice #14776

Open
MadLittleMods opened this issue Jan 4, 2023 · 0 comments
Open
Labels
A-Application-Service Related to AS support A-Performance Performance, both client-facing and admin-facing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Jan 4, 2023

Spawning from the PoC in #14729

Problem

Currently, when an application service (appservice) creates an event, if the appservice is also "interested", we also send that event back out to the appservice (echo). This echo is normally ignored by bridge libraries like with the suppressEcho option in matrix-appservice-bridge. But there is significant overhead when it comes to ignoring all of those transactions on the client and even Synapse itself can get backed up with events it doesn't even necessarily need to send out. Instead of having the round-trip just to ignore, we can ignore directly in Synapse to not send it out via the appservice.

We were seeing this problem in real-life with the Gitter -> Matrix import process and made a quick patch to get around this problem. @turt2live also shared a similar t2bot.io patch that they've been running for a long-time: t2bot@3c1cd9a

Potential solution

There should be a way to ignore events that are created by an appservice to not go back out the appservice that sent it. Or ideally, this behavior should be a sane default. This probably requires a MSC to define the behavior in the AS registration config.

@Half-Shot shared MSC2487 which is related but goes beyond the scope that I care about and given the extra complexity, extra bikeshedding will ensue. Probably best to create a new MSC.

Potential alternatives

One alternative in the Gitter -> Matrix import scenario is to have something like #3237 which ignores any old events which would work perfectly fine since we're only importing old messages. And on the Gitter side of the bridge, we already ignore any events older than 30 minutes anyway so a solution to that separate issue makes sense in any case. But we should also handle the case for new/any events which this issue addresses.

@MadLittleMods MadLittleMods added the A-Application-Service Related to AS support label Jan 4, 2023
@DMRobertson DMRobertson added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. A-Performance Performance, both client-facing and admin-facing S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support A-Performance Performance, both client-facing and admin-facing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

2 participants