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

room_stats_historical and user_stats_historical appear unused #9602

Closed
richvdh opened this issue Mar 12, 2021 · 2 comments · Fixed by #9721
Closed

room_stats_historical and user_stats_historical appear unused #9602

richvdh opened this issue Mar 12, 2021 · 2 comments · Fixed by #9721
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@richvdh
Copy link
Member

richvdh commented Mar 12, 2021

We have a whole lot of code which maintains the room_stats_historical and user_stats_historical tables (and a config option, bucket_size, which configures how it is populated), but afaict they are never actually used.

It's not obvious to me what the purpose of these tables are for, and I'd argue that unless we have a clear usecase for them, the tables and dead code should be stripped out.

It looks like the concept of historical stats was introduced by @ara4n in the initial commits in #4338, though it subsequently got refactored out of recognition by #5971. @ara4n: any comments on the purpose of this, and/or objections to it being ripped out.

Also of interest (and would need updating) is the doc, https://github.com/matrix-org/synapse/blob/v1.29.0/docs/room_and_user_statistics.md.

@richvdh
Copy link
Member Author

richvdh commented Mar 12, 2021

another point on this: according to the doc, it was originally envisaged that we would expire old entries from the historical tables. That code has never been written, and meanwhile this data is taking up lots of space on peoples' databases.

@clokep clokep added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Mar 12, 2021
@clokep clokep mentioned this issue Jun 7, 2021
7 tasks
anoadragon453 added a commit to Sorunome/synapse that referenced this issue Jun 8, 2021
room_stats_historical doesn't appear to be ever read from. See matrix-org#9602.
anoadragon453 added a commit to Sorunome/synapse that referenced this issue Jun 9, 2021
This table's usefulness is debatable (see matrix-org#9602), but is currently used by both
the codebase and tests. Thus for now I'm leaving it in, but it may well
be removed in a future PR.
@anoadragon453
Copy link
Member

I thought during #6739 that we actually did use this table as we make a select from it here:

slice_list = self.db_pool.simple_select_list_paginate_txn(
txn,
table + "_historical",
"end_ts",
start,
size,
retcols=selected_columns + ["bucket_size", "end_ts"],
keyvalues={id_col: stats_id},
order_direction="DESC",
)

but it turns out that function is only called from the tests. So indeed, we don't seem to use this table in practice (though there are a few tests that mandate it be kept up to date currently).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants