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

Hierarchy pagination is limited to 50 items per space #11134

Closed
aofn opened this issue Oct 20, 2021 · 7 comments · Fixed by #11695
Closed

Hierarchy pagination is limited to 50 items per space #11134

aofn opened this issue Oct 20, 2021 · 7 comments · Fixed by #11695
Assignees
Labels
A-Spaces Hierarchical organization of rooms T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@aofn
Copy link

aofn commented Oct 20, 2021

_matrix/client/unstable/org.matrix.msc2946/rooms/{roomId}/hierarchy returns a next_batch string for spaces with more than 50 children as expected.
Pparsing the pagination token then only returns 11 rooms in my case, even though there should be much more. The Space in question has 135 space children.
Different spaces return different seemingly random numbers after the second call, but never the actual number.

To Reproduce
Steps to reproduce the behavior:

  • call _matrix/client/unstable/org.matrix.msc2946/rooms({roomId}/hierarchy for a space with more than 50 children, ideally more than 100

  • repeat the call and parse the next_batch string

  • wrong number of spaces are returned

Expected behavior

  • return actual number of spaces and if >50 another next_batch string

tested with v1.43.0

@aofn aofn changed the title API _matrix/client/unstable/org.matrix.msc2946/rooms({roomId}/hierarchy pagination broken API _matrix/client/unstable/org.matrix.msc2946/rooms/{roomId}/hierarchy pagination broken Oct 20, 2021
@squahtx squahtx added T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. A-Spaces Hierarchical organization of rooms X-Needs-Discussion labels Oct 20, 2021
@squahtx
Copy link
Contributor

squahtx commented Oct 20, 2021

Are you always seeing the same incorrect number of rooms returned, or does it differ between pagination attempts? And is it the same subset of rooms returned every time?

What's the structure of the space with 135 children? ie. how many rooms and subspaces are there in each subspace?

@clokep
Copy link
Contributor

clokep commented Oct 20, 2021

Is this a public space? If so, can you provide the room ID?

@squahtx squahtx added X-Needs-Info This issue is blocked awaiting information from the reporter and removed X-Needs-Discussion labels Oct 20, 2021
@aofn
Copy link
Author

aofn commented Oct 20, 2021

Are you always seeing the same incorrect number of rooms returned, or does it differ between pagination attempts?

Yes, I always get the same incorrect number of rooms returned for a specific space (but the number varies from one space to another).

And is it the same subset of rooms returned every time?

Yes, always the same rooms.

What's the structure of the space with 135 children? ie. how many rooms and subspaces are there in each subspace?

The room structure is as follows:

parent space
  | children space 1
  | children space 2
  | children space 3
  ...
  | children space 134
  | children space 135

no further sub-spaces

Is this a public space? If so, can you provide the room ID?

I've created a public dummy space with 120 space children here: !iUCIUjePntDkDGSnjb:matrix.org
In this case the second API call with the pagination token only returns one room.

edit: just to verify, GET https://matrix.org/_matrix/client/r0/rooms/!iUCIUjePntDkDGSnjb:matrix.org/state
has 120 events of the typem.space.child

@squahtx squahtx removed the X-Needs-Info This issue is blocked awaiting information from the reporter label Oct 21, 2021
@squahtx
Copy link
Contributor

squahtx commented Oct 21, 2021

I suspect what's happening here is that only the first 50 children of each subspace are being returned.
@marcel-klasse does this line up with what you're seeing?

@aofn
Copy link
Author

aofn commented Oct 21, 2021

that's possible. I have just added 5 children to the first subspace like this:

parent space
  | children space 1
     | child 1
     | child 2
     | child 3
     | child 4
     | child 5
  | children space 2
  | children space 3
  ...
  | children space 119
  | children space 120

which is now returning 4 rooms after the second call.

@zv1n
Copy link

zv1n commented Nov 22, 2021

I'm seeing this exact issue when trying to migrate a list of rooms from another service over to a matrix server space.

The issue appears to be that the 50 room limit is hard coded on line 52 in handlers/room_summary.py with the request cap limiting logic appearing on line 653 in handlers/room_summary.py.

The limiting logic appears to come in from PR Federation API for Space summary (#9652) where the max_children check against MAX_ROOMS_PER_SPACE is first introduced into _summarize_local_room.

I've tested this on a local test VM. When I set this variable (MAX_ROOMS_PER_SPACE) to 100, it works as expected (I have 90 rooms and got 90 rooms).

The easiest solution would seem to be expose this as a configuration variable so individual servers can override the limit as needed, but I figured I'd drop my research in for the synapse team to consider.

Thanks!

@clokep clokep changed the title API _matrix/client/unstable/org.matrix.msc2946/rooms/{roomId}/hierarchy pagination broken Hierarchy pagination is limited to 50 items per space Nov 23, 2021
@clokep
Copy link
Contributor

clokep commented Nov 23, 2021

I've reworded the issue since it isn't really broken, but working as designed. Part of the problem with this code right now is that it is trying to support both the legacy /spaces API and the newer /hierarchy API so it gets a bit messy. We could likely up the limit of 50 higher, but it likely makes more sense to ensure we can handle arbitrary limits. I would need to look deeper into the code to see how easy that is.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spaces Hierarchical organization of rooms T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants