Stabilize support for MSC3952: intentional mentions

[clokep]

MSC3952 has almost passed FCP, we need a plan to stabilize it.

• Swap unstable prefixes for stable ones.
• Figure out if we can send the new rule down incremental syncs.

[clokep]

I think in the past we've just punted on this problem, but does there seem like a reasonable way to have clients get notified of a new push rule for incremental syncs?

I think we'd need to do something like push a new row into push_rules_stream for each user? This sounds quite expensive, but I think we could do it in the background (with the caveat that for a period of the time the homeserver and client might disagree on the push rules, but this is going to happen anyway with old clients).

What do people think about this?

[clokep]

I think we'd need to do something like push a new row into push_rules_stream for each user?

Actually, we might need to add two rows, one for .m.rule.is_user_mention and one for .m.rule.is_room_mention.

[clokep]

@reivilibre had a few good ideas:

• Capture the "latest" push rule stream ID somewhere else and use it in sync.
• Add a "system" stream table that we use for migrations like this.
• Add a row without a user_id in push_rules_stream.

Other thoughts:

• Stream writers are the only things that should write to stream tables.
• What happens if the server rolls back and then forwards again?
• Double check if clients refresh push rules during start-up.
• Ensure that the new conditions / rules don't break clients dramatically.

[clokep]

Ensure that the new conditions / rules don't break clients dramatically.

I asked client developers for some help with this. The matrix-js-sdk treats unknown conditions as "not matching" which seems like a reasonable way to fallback.

[clokep]

Ensure that the new conditions / rules don't break clients dramatically.

I asked client developers for some help with this. The matrix-js-sdk treats unknown conditions as "not matching" which seems like a reasonable way to fallback.

Turns out that we've been leaking the extensible events rules since they were implemented, which have new conditions and nothing has exploded. 😢 See #15494.

[clokep]

Add a row without a user_id in push_rules_stream.

I think I have a proof-of-concept using this approach, it does mean that we have to make the user_id column nullable (I thought it was already, but I was incorrect). I'm not sure how much of a pain this is vs. potentially the idea of a "system upgrade" stream table.

Stream writers are the only things that should write to stream tables.

It seems like the push rules stream is only writable from the main process (which is also where we run background updates), so I don't think this will be an issue.

What happens if the server rolls back and then forwards again?

This depends a bit on the implementation, but I guess we'd write a marker that all user's streams are updated at X. After a rollback we wouldn't read that marker anywhere, but individual user streams might move past X. When rolling forward again we would read the marker, but the user's stream is farther along so we would do nothing with it?

tl;dr Rollback + forward could mean that some users aren't notified of the update.

[clokep]

Double check if clients refresh push rules during start-up.

It looks like Element Web pulls push rules separately during an incremental sync.

---

I discussed this a bit with folks and I think the failure mode of the push rules being out of date between server & client is that you end up in backwards compatiblity discussed in the MSC. It seems any attempt to push the rules down a stream will probably be a bit buggy, which makes me think it won't be worth the effort.

[clokep]

We'll want to tweak this a bit more per this comment to handle a better migration path for users from the legacy rules to the stable rules.

[clokep]

We'll want to tweak this a bit more per this comment to handle a better migration path for users from the legacy rules to the stable rules.

This looks a bit more annoying than expected, since the push_rule_enable uses an ID generator we'll need a background update to do it. This is awkward since the user might have notifications from the new rule before the update runs. I can't think of a good way around this though.

Additionally things that won't work well include:

• Users on old clients disabling the legacy rule only.
• Rollback and then rollforward of a deploy (i.e. this would be a 1-time transition).

[daniellekirkwood]

We no longer want to migrate existing users.

@clokep did an analysis of how many folks would need to be migrated and the amount of effort it would take to do so -- I do not believe it's worth it and the outcome of not migrating people is not that bad (they can go back to settings and turn it off).

Therefore, this is ready to go!

The meta issue has also been updated FYI: element-hq/element-web#24291

[daniellekirkwood]

@pmaier1 I believe that @clokep has completed all the work needed here (especially now we've removed the migration piece) would you help me in identifying next steps to closing this out? It maybe just needs a peer-review?

[clokep]

I think we just need to merge #15520.