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

Reduce performance logging to DEBUG #6833

Merged
merged 2 commits into from
Feb 5, 2020

Conversation

michaelkaye
Copy link
Contributor

@michaelkaye michaelkaye commented Feb 3, 2020

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.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

This PR reduces the default logging level of verbose performance monitoring counters from INFO to DEBUG.

Leo even wrote along side this code:

        # TODO(paul): These can eventually be removed once the metrics code
        #   is running in mainline, and we have some nice monitoring frontends
        #   to watch it

So there's definitely a case to rip this out entirely - reducing to DEBUG seems safe.

These metrics are available more generally in the prometheus metrics (eg synapse_background_process_db_txn_duration_seconds) which when looking at performance changes over time is probably a better way to access this data.

This change originated in feedback that these messages are confusing to server administrators, especially at the INFO level.

@michaelkaye michaelkaye force-pushed the michaelkaye/synapse.storage.TIME_log_level branch from 62e4923 to 0cf32bd Compare February 3, 2020 15:00
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm, though I wonder if we should get rid of all the spurious _txn_perf_counters stuff.

@michaelkaye
Copy link
Contributor Author

lgtm, though I wonder if we should get rid of all the spurious _txn_perf_counters stuff.

I would not be opposed to that, but maybe a larger change. I'll open an issue

@michaelkaye michaelkaye merged commit a831d2e into develop Feb 5, 2020
@@ -0,0 +1 @@
Reducing log level to DEBUG for synapse.storage.TIME.
Copy link
Member

Choose a reason for hiding this comment

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

quick reminder that changelogs should be in the imperative mood: Reduce log level to DEBUG for synapse.storage.TIME.

babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'a831d2e4e':
  Reduce performance logging to DEBUG (#6833)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants