Replace master/slave terminology with something more appropriate

Bug #1882937 reported by Jane Sandberg on 2020-06-10
20
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Undecided
Unassigned

Bug Description

In the weeks since George Floyd was murdered, I've been seeing calls for developers to be more thoughtful about the terminology they use. Master/slave has oppressive connotations, and several other software communities have decided that it is an inappropriate metaphor. Evergreen uses it in one file: https://github.com/evergreen-library-system/Evergreen/blob/master/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg.pm

Primary/replica is a widely-used alternative. Primary/standby is another alternative with some Postgres flavor.

It's a small change, but I think it is overdue.

Shula Link (slink-g) on 2020-06-10
Changed in evergreen:
status: New → Confirmed
Shula Link (slink-g) wrote :

Agreed, confirmed, possible patch pushed to https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=926eec6d800eeade2d5b4eae876e5bfc4d9e1dc1 with primary/standby terminology.

Shula Link (slink-g) on 2020-06-10
tags: added: pullreq
tags: added: pullrequest
removed: pullreq
Dan Wells (dbw2) wrote :

I mean this with all possible sincerity and gentleness, but this is a really silly thing to change. I definitely do not support making any changes like this, now or ever. The value is just so low (it isn't even public), and the chances of breaking something doing a "simple" mass-replace are non-zero. In fact, the patch here would break something were it not for the fact that the broken code is already commented out.

I see this similar to how I see the use of "daemon" (though the way that term came about is somewhat convoluted) and some other sketchy terms used throughout computing and engineering. They may be done in poor taste, but certainly not malicious. Computer programmers are not the most tasteful bunch, and never will be, and clarity trumps everything. As well it should, since it is so darn hard to make things clear.

Finally, distractions such as this bug also contribute to the harm caused by this manner of oversensitive thinking, first preventing you and now preventing me from doing something of shared value. It is a road best not taken.

Jane Sandberg (sandbej) wrote :

The best way to make sure we don't break anything is good automated testing. I'd be happy to write some tests for this module, which would be valuable any time we want to refactor or change this code.

There are countless arguments online about how clear the master/slave terminology actually is in this context; I really don't think that it is a given. I agree that its use here was not done with malicious intent, but that doesn't mean it shouldn't change.

Terran McCanna (tmccanna) wrote :

I agree with Jane - testing is critical, but that doesn't mean it shouldn't be done.

This terminology has been controversial for a very long time, it's not just suddenly an issue. Drupal changed it in 2014, and Python changed it in 2018: https://www.vice.com/en_us/article/8x7akv/masterslave-terminology-was-removed-from-python-programming-language

Yamil (ysuarez) wrote :

@Jane you know so much more than me about Evergreen at this point, but let me know if I can provide any help with writing the automated tests that would be needed.

Shula Link (slink-g) wrote :

Also, I only did a search-and-replace after going through the code and verifying that all the instances of master/slave were local variables and comments. Had it been otherwise, I would have done a more precise patch - however, as it stands, the code should function, though it could certainly be made nicer.

Changed in evergreen:
assignee: nobody → Mike Rylander (mrylander)
Mike Rylander (mrylander) wrote :

Indeed, the terminology in the module derives from terms used in Class::DBI::Replication. I was attempting to use Someone Else's Code to simplify my life, but it was abandonware even then and I ended up implementing my own multi-db-backend setup within 4 days.

Cribbing from preexisting code (that we ended up not using, anyway) doesn't chisel it in stone, and it should be changed. Sam's patch is a good improvement.

Regarding the risk of bugs, I wrote the original so I'll take any flack for breakage. It took more time for me to read this bug than to vet the patch...

Thanks, Sam.

Pushed to the master branch with a little additional cleanup to remove confusing comments related to the unimplemented thought of using C::D::Replication, and the commented-out code that Dan mentioned.

Changed in evergreen:
assignee: Mike Rylander (mrylander) → nobody
Changed in evergreen:
status: Confirmed → Fix Committed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers