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

use_presence: Default to False #9251

Closed
wants to merge 1 commit into from
Closed

use_presence: Default to False #9251

wants to merge 1 commit into from

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 28, 2021

This is a pre-emptive measure so that a user's first install experience is smoother.

Tracking issue with use_presence's performance problem:
#3971.

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)

Tracking issue with use_presence's performance problem:
matrix-org#3971.

Signed-off-by: rht <rhtbot@protonmail.com>
@rht
Copy link
Contributor Author

rht commented Jan 30, 2021

I will fix the test cases if there is a consensus that presence should be disabled by default.

@rht
Copy link
Contributor Author

rht commented Jan 30, 2021

Actually, since this default value being set to False is temporary, there is less hassle with just saying in INSTALL.md that use_presence should be off.

@clokep
Copy link
Member

clokep commented Feb 1, 2021

I don't think we can just change the default here since it would cause people using this feature to have it automatically disabled when upgrading?

@rht
Copy link
Contributor Author

rht commented Feb 1, 2021

Yeah, better if just recommend in INSTALL.md to turn it off for now?

@clokep
Copy link
Member

clokep commented Feb 3, 2021

Yeah, better if just recommend in INSTALL.md to turn it off for now?

This is mentioned in the README already: https://github.com/matrix-org/synapse#help-synapse-is-slow-and-eats-all-my-ramcpu

All of this could be a bit better organized intro a troubleshooting section, but I don't think that belongs in the INSTALL file.

I'm going to close this for now since the proposed solution is not backwards compatible.

@clokep clokep closed this Feb 3, 2021
@rht
Copy link
Contributor Author

rht commented Feb 4, 2021

I have seen that text in README. Unfortunately it is a remedy after people have noticed their RAM and CPU shot up. It's better to warn people in advance in INSTALL.md.

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