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

room stats are not working for rooms created since the initial background_update #5624

Closed
richvdh opened this issue Jul 5, 2019 · 5 comments
Assignees
Labels
z-bug (Deprecated Label)

Comments

@richvdh
Copy link
Member

richvdh commented Jul 5, 2019

No description provided.

@hawkowl
Copy link
Contributor

hawkowl commented Jul 5, 2019

The problem is that

def _update_stats_txn(self, txn, stats_type, stats_id, ts, fields):
isn't being run when there's a new room
elif typ == EventTypes.Create:
.

The fix should add that, with proper values (so probably all zeroes except for the state events or whatever?)

Plus,

if len(rows) == 0:
, which let this pass by silently, should raise an exception instead, or go and calculate the room stats itself (since raising an exception would explode existing users without rerunning the background update, which sucks for matrix.org).

@richvdh
Copy link
Member Author

richvdh commented Jul 5, 2019

incidentally, there is an update_stats at

def update_stats(self, stats_type, stats_id, ts, fields):
which seems unused but should possibly be used here?

@neilisfragile neilisfragile added z-bug (Deprecated Label) z-p2 (Deprecated Label) p1 and removed z-p2 (Deprecated Label) labels Jul 9, 2019
@reivilibre reivilibre self-assigned this Jul 12, 2019
@reivilibre
Copy link
Contributor

reivilibre commented Jul 15, 2019

I've been looking into some of the code surrounding this issue.

Some notes

  • I don't think that room creation & upgrade needs any special effort — as long as update_state_deltas is updated to generate missing rows.
  • If you have room_stats for H but are missing room_stats for M,
    then after a background room_stats update, you will have room_stats for M but be missing them for H.
    (This happens because the setup code only selects rooms without stats to have stats calculated,
    but the actual process of calculating stats discards all existing stats)

Possible solutions

  1. Make room creation & upgrade insert a zero-row into room_stats AND fix the background update process so it doesn't discard existing progress.
    • This relies on us scheduling another background update (but only a partial one) in the migration SQL to fix existing installations.
  2. Make update_state_deltas calculate the stats if required instead of ignoring a lack of stats rows.
    • This would mean that current rooms missing stats would have them generated as soon as a new state event is added into them.
    • However, this means that these rooms won't appear in the room directory (after publicRooms API is repeatedly timing out on basic searches #4043) until a state event occurs.
      • For this reason, it is probably desirable to fix the background update process to not discard existing progress and also schedule one using the migration SQL.
  3. For completeness' sake, both options could be combined.

@reivilibre
Copy link
Contributor

reivilibre commented Jul 18, 2019

As part of this issue, I also need to schedule a new background update.

It's not clear (to me) whether it should discard previous room_stats or not. In general, they are likely to be wrong due to #5423 and #5690. Any thoughts on this?

I will block this background update on #5690 #5423 #5710 being resolved.

@erikjohnston
Copy link
Member

Fixed in #5971

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

5 participants