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

The limit set for a filter in a room's StateFilter is ignored #7306

Open
flaki opened this issue Apr 19, 2020 · 5 comments
Open

The limit set for a filter in a room's StateFilter is ignored #7306

flaki opened this issue Apr 19, 2020 · 5 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec A-Sync defects related to /sync O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@flaki
Copy link

flaki commented Apr 19, 2020

Description

I'm using the below filter in a sync:

{
  "room": {
    "state": {
      "types": [
        "m.room.member",
        "m.room.name"
      ],
      "limit": 10
    },
    "timeline": {
      "limit": 10
    },
    "ephemeral": {
      "not_types": [ "*" ]
    }
  }
}

Please note that both room state and timeline limits are set to 10. In the resulting JSON I see my limit applied in timeline events (also limited: true), but not in room state events (one room in join has 4000+ events listed). According to the client-server API docs a limit clause should apply to StateFilter

Steps to reproduce

  • perform a sync without since
  • include a filter in the query that requests room.state.limit = 10

The limit should apply in e.g. rooms.join.<roomId>.state.events, but it doesn't seem to do so.

  • Homeserver: mozilla.modular.im

If not matrix.org:

  • Version: {"server_version": "1.12.3", "python_version": "3.7.7"}

  • Install method:

  • Platform:
@clokep
Copy link
Member

clokep commented Apr 20, 2020

Edited: Arg, what I said didn't make any sense -- lined up the keys for your example JSON wrong.

@clokep clokep added z-bug (Deprecated Label) z-p2 (Deprecated Label) A-Spec-Compliance places where synapse does not conform to the spec labels Apr 20, 2020
@clokep
Copy link
Member

clokep commented Apr 20, 2020

Looking a bit into the filtering code it does seem like the state key's limit is never pulled out.

FilterCollection._room_state_filter never has limit() called on it.

@KB1RD
Copy link
Contributor

KB1RD commented Apr 20, 2020

AFAIK, the limit is not respected for presence events, except if it's zero. I noticed that in filtering, it looks like the call to get_new_events has no limit applied to it.

@flaki
Copy link
Author

flaki commented Apr 21, 2020

I initially set out with very similar predisposition as @turt2live in #4299 -- I just wanted a very specific sync with only very limited data, and to cut down on the several-megabyte JSONs downloaded.

I think efficient filtering is super important if we want a thriving client ecosystem, even if, for now that just means writing up a guide on /develop so devs setting out writing a new client are not inundated with the deep rabbit hole of Filters right away (and straight up give up on efficient filtering, hurting both the server and the user's bandwidth/performance).

But of course, step 0 to all that is having a server that work as specced. ☺️

@D00mch
Copy link

D00mch commented Nov 4, 2022

I notice that "limit" doesn't influence the number of events I get from sync. For example, I get all the events (as if I don't have a filter at all) having filter like this:

{
  "account_data": {
    "limit": 0
  },
  "presence": {
    "limit": 0
  },
  "room": {
    "ephemeral": {
      "limit": 0
    },
    "state": {
      "limit": 0
    },
    "timeline": {
      "limit": 0
    },
    "account_data": {
      "limit": 0
    }
  }
}

To force sync response to be empty, I have to add empty "types": [ ]

And I also found that just types without limit return empty response but it take 2 times longer.

{
  "server_version": "1.69.0",
  "python_version": "3.9.15"
}

@squahtx squahtx added A-Sync defects related to /sync S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. and removed z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec A-Sync defects related to /sync O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

5 participants