Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify use of read replicas #6588

Merged
merged 7 commits into from
Aug 22, 2023
Merged

Conversation

grubbins
Copy link
Contributor

The docs suggest that reads are evenly distributed between the master DB and the read replica DBs. Based on both the server code and the observed behaviour, I do not believe that is the case - I believe that (if any read replicas are available) read queries are distributed among the read replicas only. This is a useful distinction to keep in mind for site admins.

The docs suggest that reads are evenly distributed between the master DB and the read replica DBs. Based on both the server code and the observed behaviour, I do not believe that is the case - I believe that (if any read replicas are available) read queries are distributed among the read replicas only. This is a useful distinction to keep in mind for site admins.
@mattermost-build
Copy link
Contributor

Hello @grubbins,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@cwarnermm cwarnermm added 1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor labels Aug 17, 2023
Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

The original text is technically correct. While it's true that 99% of times, read requests are sent to the read replicas, there are cases where they aren't. If you want to change the text to reflect this, we can do it. Otherwise we can leave it as is.

@grubbins
Copy link
Contributor Author

Yes - I think there's a big difference between "Mattermost distributes read requests across all databases", and "Mattermost distributes almost all read requests across the read replicas only". Would indeed like to see an update...

Are you asking me to propose different wording?

@cwarnermm
Copy link
Member

@grubbins - We'd welcome your docs contribution with updated, clarified wording if you're open to it.

@grubbins
Copy link
Contributor Author

OK - please see what you think of my latest wording.

@cwarnermm cwarnermm self-requested a review August 22, 2023 12:56
Copy link
Member

@cwarnermm cwarnermm left a comment

Choose a reason for hiding this comment

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

Thank you so much, @grubbins!

@cwarnermm cwarnermm added 3: Reviews Complete All reviewers have approved the pull request and removed 1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor labels Aug 22, 2023
@cwarnermm cwarnermm merged commit d8ddc8c into mattermost:master Aug 22, 2023
3 checks passed
@grubbins
Copy link
Contributor Author

Oops - I think the markdown requires a linebreak before the bullet points. The html currently up on the docs page has them as inline asterisks.

Could you insert the linebreak @cwarnermm ? Or do you need me to open a new PR with it?

@cwarnermm
Copy link
Member

I'm happy to update the formatting, @grubbins!

@cwarnermm cwarnermm mentioned this pull request Aug 28, 2023
@cwarnermm
Copy link
Member

Fixed via #6627

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request Contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants