SSS appears to burn huge amounts of bandwidth returning obsolete `ops` fields

[ara4n]

Description

While hacking on aurora (EWX), I made the mistake of looking in the network inspector /sync responses and discovered that the entire roomlist was being returned in every single Simplified Sliding Sync (SSS) response in an ops field. As a result, every single long poll is burning an additional 10s of kilobytes of response re-transferring every room Id you're in(!)

However, SSS doesn't even define an ops field in the MSC - this was the main thing we removed from sliding sync when we simplified it into SSS.

So I assume this is a weird thinko throwback which can & should be removed?

Steps to reproduce

• list the steps
• that reproduce the bug
• using hyphens as bullet points

Homeserver

matrix.org

Synapse Version

1.131.0rc1 (b=matrix-org-hotfixes-priv,68afa35f06)

Installation Method

Other (please mention below)

Database

postgres

Workers

Multiple workers

Platform

deb

Configuration

No response

Relevant log output

{
  "pos": "78864226/m5961565550~2.5961565552~37.5961565552_757284974_40074560_3698127888_4163553842_265824357_1495132460_11139272087_0_492038",
  "lists": {
    "all_rooms": {
      "count": 153,
      "ops": [
        {
          "op": "SYNC",
          "range": [
            0,
            152
          ],
          "room_ids": [
            "!uPnuYllGuuELYxjtma:matrix.org",
            /* ...150 room ids... */
            "!SwmvnUnaSDxPdvRevr:matrix.org",
          ]
        }
      ]
    }
  },
  "rooms": {
    "!OGEhHVWSdvArJzumhm:matrix.org": {
        ...
    }
  },
  "extensions": {
    "account_data": {
      "global": [],
      "rooms": {}
    },
    "receipts": {
      "rooms": {}
    },
    "typing": {
      "rooms": {}
    }
  }
}

Anything else that would be useful to know?

No response

[reivilibre]

confirmed — also checked Ruma doesn't rely on them, so really don't think anyone wants them — seems easy enough to strip them out from the main code, but the tests do depend on them to quite a large extent for checking sliding sync is choosing the right rooms to deal with :(

It might be time to consider some kind of probe point in Synapse that tests can use to check what sliding sync is thinking internally, but not sure.

[ara4n]

i'm expecting that non-simplified-sliding-sync will return (for low-bandwidth matrix) in future, so avoiding removing the ops entirely could be good.

how about keeping them if you say io.element.slidingsync.ops: true on the request or something, and then the tests can depend on them as an inspection mechanism, and everyone wins?

[MadLittleMods]

See #17671 for previous discussions around this. I think it still illustrates my thoughts:

• For clients that use multiple lists, they need some know which rooms apply to which lists. Currently, this comes from a hard-coded SYNC operation with the room_ids but could simplified to just a simple list of room_ids.
• If bandwidth and size bulk for the list of room_ids is really a concern, it could be an extension

[Hywan]

I believe providing this information as an extension is pretty clever. I reckon it even is probably better than the io.element.slidingsync.ops: true proposal from @ara4n (no offense): sliding sync provides an extension mechanism, let's use it!

About:

For clients that use multiple lists, they need some know which rooms apply to which lists.

Yes and no. We (the Matrix Rust SDK) were using multiple lists in the past, and this specific need never popped to be honest.

I think this case is so specific that it deserves to be moved in an extension. That way, the test suite isn't broken (because you can activate this extension everywhere in your tests), and this feature isn't thrown away. One rock, two hits (let's be nice with birds).

Edit: the extension can straightforwardly return the set of updated rooms per list, without an operation. So far the only supported operation is SYNC, which is then unnecessary since it's the only one.

[reivilibre]

Maybe the rooms entries should provide the list IDs (or indices) that the room is for? Seems like it might provide an easy way to stop sending down the full list each time whilst not imposing any more bookkeeping on the server.

[ara4n]

perfectly happy to have it as an extension - would be really nice as a way to extend SSS back into SS when needed.

please can we do it? the bandwidth hit currently is a real mess