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

Fix #397: Change master/slave to primary/replica #217

Closed
wants to merge 1 commit into from
Closed

Fix #397: Change master/slave to primary/replica #217

wants to merge 1 commit into from

Conversation

chriscoffee
Copy link

@chriscoffee chriscoffee commented Sep 22, 2018

This is somewhat of a tricky pull request that with all honesty I don't expect to get merged.

There are still some that feel that there are benifits of using the 'well-known' terminology of master and slave far rather than those without negative historic connotations such as leader/follower or primary/replica citing readability etc..

However, I hope you will atleast consider seriously altering the language in this repository, especially if it's helpful as a tool for interviews to work on large-scale systems given many companies have adopted either leader/follower or primary/replica, as alternate terms of reference.

Some examples its alteration, and conversation around it can be found below. I have tried to find some who haven't made the move in order to provide a balanced argument, feel free to add more in the comments:

I think even if there's just a footnote referencing that some feel that this is no longer an acceptable reference, that's also an option.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@utako
Copy link

utako commented Jul 26, 2020

This really needs to get updated and merged -- the master-slave terminology is distracting and unnecessary and pretty tone-deaf in this day and age. @donnemartin

@NiklasBr
Copy link

NiklasBr commented Jul 26, 2020

Doctrine/DBAL did the change this summer: RFC: Rename MasterSlaveConnection #4052

Wordpress is in the process of dropping it: Rename master branch to some other naming (and other problematic words)

@chriscoffee
Copy link
Author

I'd completely forgotten about this. I'm happy to update 👍

Copy link
Owner

@donnemartin donnemartin left a comment

Choose a reason for hiding this comment

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

Hi @chriscoffee, appreciate the PR. Could we make the following changes? Thank you!

  • Use the terms primary/replica.
  • Not change the images and OmniGraffle templates. Some are binary it's hard for me to review the diffs. I'm planning on making other changes to images so I'm happy to make these updates.
  • Resolve merge conflicts.

@donnemartin donnemartin changed the title Consider leader/follower over master/slave Fix #397: Change master/slave to primary/replica Aug 2, 2020
After further feedback I've not updated the images and just made
alterations to the READMEs for mentions of master-slave to
primary-replica
@chriscoffee
Copy link
Author

@donnemartin Ok should be sorted. Let me know if you need anything else 👍

As requested I've just updated the README's (including references in ZH versions) and resolved merge conflicts. Apologies for forced pushes, I swapped out old commits for correctly authored signed commits under my actual user name. Initially I leaned on Leader/Follow as statements like this felt somewhat clunky to read:

It's important to discuss what bottlenecks you might encounter ... For example, what issues are addressed by adding a Load Balancer with multiple Web Servers? CDN? Primary-Replica Replicas? ...

So could need re-working maybe?

Also I don't know if you're the sole maintainer on this project, but if you go down the route of swapping master -> main I would be happy to help out!

@utako
Copy link

utako commented Aug 5, 2020

Thank you!

Copy link
Owner

@donnemartin donnemartin left a comment

Choose a reason for hiding this comment

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

@chriscoffee thanks for the updated PR!

this felt somewhat clunky to read...Primary-Replica Replicas

I agree, I added wording change suggestions.

The PR seems to only address the solutions, are you able to update the main README?

@@ -333,7 +333,7 @@ class SpendingByCategory(MRJob):

State you would 1) **Benchmark/Load Test**, 2) **Profile** for bottlenecks 3) address bottlenecks while evaluating alternatives and trade-offs, and 4) repeat. See [Design a system that scales to millions of users on AWS](../scaling_aws/README.md) as a sample on how to iteratively scale the initial design.

It's important to discuss what bottlenecks you might encounter with the initial design and how you might address each of them. For example, what issues are addressed by adding a **Load Balancer** with multiple **Web Servers**? **CDN**? **Master-Slave Replicas**? What are the alternatives and **Trade-Offs** for each?
It's important to discuss what bottlenecks you might encounter with the initial design and how you might address each of them. For example, what issues are addressed by adding a **Load Balancer** with multiple **Web Servers**? **CDN**? **Primary-Replica Replicas**? What are the alternatives and **Trade-Offs** for each?
Copy link
Owner

Choose a reason for hiding this comment

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

Can we change all instances of "Primary-Replica Replicas" to "Primary-Replicas"? Any instances where this would read awkwardly?

* [SQL write master-slave failover](https://github.com/donnemartin/system-design-primer#fail-over)
* [Master-slave replication](https://github.com/donnemartin/system-design-primer#master-slave-replication)
* [SQL write primary-replica failover](https://github.com/donnemartin/system-design-primer#fail-over)
* [Primary-replica replication](https://github.com/donnemartin/system-design-primer#primary-replica-replication)
Copy link
Owner

Choose a reason for hiding this comment

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

Can we change this from "Primary-replica replication" to "Primary-replica"? Same with anchors. Any instances where this would read awkwardly?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants