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

Simplify query to _count_known_servers() for the synapse_federation_known_servers metric #15735

Closed
wants to merge 3 commits into from
Closed

Simplify query to _count_known_servers() for the synapse_federation_known_servers metric #15735

wants to merge 3 commits into from

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Jun 7, 2023 •
edited
Loading

Simplify query to _count_known_servers() for the synapse_federation_known_servers metric

Spawning from #15731

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

@MadLittleMods MadLittleMods added T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. A-Metrics metrics, measures, stuff we put in Prometheus A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db labels Jun 7, 2023
MadLittleMods
MadLittleMods commented Jun 7, 2023
View reviewed changes
synapse/storage/databases/main/roommember.py
Comment on lines -136 to -137
INNER JOIN current_state_events as c ON rm.event_id = c.event_id
WHERE c.type = 'm.room.member'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If #15734 (comment) is valid, then I think this simplification is also valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a valid change, see #15734 (comment)

@MadLittleMods MadLittleMods mentioned this pull request Jun 7, 2023
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db A-Metrics metrics, measures, stuff we put in Prometheus T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@MadLittleMods