Remove `master` and `slave` from source code #591

PingXie · 2024-06-03T05:07:50Z

External facing interfaces are not affected.

Signed-off-by: Ping Xie <pingxie@google.com>

codecov · 2024-06-03T05:19:42Z

Codecov Report

Attention: Patch coverage is 75.51440% with 119 lines in your changes missing coverage. Please review.

Project coverage is 70.26%. Comparing base (bce240e) to head (625d764).

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #591      +/-   ##
============================================
+ Coverage     70.21%   70.26%   +0.04%     
============================================
  Files           110      110              
  Lines         59994    60007      +13     
============================================
+ Hits          42124    42161      +37     
+ Misses        17870    17846      -24

Files	Coverage Δ
src/blocked.c	`91.45% <100.00%> (ø)`
src/cluster_legacy.c	`86.44% <ø> (+0.01%)`	⬆️
src/db.c	`88.32% <100.00%> (ø)`
src/debug.c	`53.94% <100.00%> (ø)`
src/evict.c	`97.34% <100.00%> (ø)`
src/hyperloglog.c	`91.21% <ø> (ø)`
src/logreqres.c	`72.72% <ø> (ø)`
src/replication.c	`87.17% <ø> (+0.08%)`	⬆️
src/script.c	`86.61% <100.00%> (ø)`
src/script_lua.c	`90.14% <ø> (ø)`
... and 16 more

... and 9 files with indirect coverage changes

hwware · 2024-06-03T14:58:56Z

Before begin to take look this huge PR, can we setup several step for this. My suggestion as below:

Set master as primary the alias name (a new big change, but still backward compatible )
Remove master and slave from source code
Update some json files, output message, and logs
Update .md file and our valkey.io website

I suggest even this PR is approved, do not merge yet until step 1 is done.

PingXie · 2024-06-03T16:38:58Z

Before begin to take look this huge PR, can we setup several step for this. My suggestion as below:

Set master as primary the alias name (a new big change, but still backward compatible )

Remove master and slave from source code

Update some json files, output message, and logs

Update .md file and our valkey.io website

I suggest even this PR is approved, do not merge yet until step 1 is done.

@hwware I think 2 should be done first and this is what this PR is about. It should not impact any code behavior. thoughts?

PingXie · 2024-06-03T16:44:27Z

Also I don't think we can do much about 1 and 3 in the foreseeable future so really the order IMO is

2,4,1,3

with 2 being the safest and should be done sooner rather than later (due to the sheer size of the change).

hwware · 2024-06-03T16:48:43Z

Before begin to take look this huge PR, can we setup several step for this. My suggestion as below:

Set master as primary the alias name (a new big change, but still backward compatible )

Remove master and slave from source code

Update some json files, output message, and logs

Update .md file and our valkey.io website

I suggest even this PR is approved, do not merge yet until step 1 is done.

@hwware I think 2 should be done first and this is what this PR is about. It should not impact any code behavior. thoughts?

My concern is developers have no idea where the term "primary" comes from (although most people can understand, primary is almost master)

PingXie · 2024-06-03T16:57:32Z

My concern is developers have no idea where the term "primary" comes from (although most people can understand, primary is almost master)

We did document it in the source code already -

valkey/src/valkeymodule.h

Line 10 in 4176604

    
            * - Terminology has been updated to be more inclusive: "slave" is now "replica", and "master"

But yeah I think we should scrub the documentation too

hwware · 2024-06-03T17:07:26Z

No problem, let us first go through this pr and then discuss other steps.

hwware · 2024-06-03T17:42:25Z

src/aof.c

@@ -1073,7 +1073,8 @@ void flushAppendOnlyFile(int force) {
             * (because there's no reason, from the AOF POV, to call fsync) and then WAITAOF may wait on


I notice in this pr, you update comment as well, pls update the above line master_repl_offset to primary_repl_offset

hwware · 2024-06-03T17:44:10Z

src/aof.c

@@ -1365,9 +1366,9 @@ struct client *createAOFClient(void) {
     */


pls update line 1358 "master"

hwware · 2024-06-03T17:45:05Z

src/aof.c

@@ -2408,7 +2409,7 @@ int rewriteAppendOnlyFileBackground(void) {



Signed-off-by: Ping Xie <pingxie@google.com>

) Make the one backwards compatible config change we are allowed to replace for removing master from our API. `masterauth` and `masteruser` are still used as an alias, but aren't explicitly referenced. As an addendum to #591, it would be good to have this in 8. Given the related PR for updated other references for master, I just updated the ones around this specific change. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

Signed-off-by: Ping Xie <pingxie@google.com>

hwware · 2024-06-07T21:19:00Z

Let us merge and then document it.

…alkey-io#598) Make the one backwards compatible config change we are allowed to replace for removing master from our API. `masterauth` and `masteruser` are still used as an alias, but aren't explicitly referenced. As an addendum to valkey-io#591, it would be good to have this in 8. Given the related PR for updated other references for master, I just updated the ones around this specific change. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com> Signed-off-by: Adva Moshkovitz <advamo7@gmail.com>

External facing interfaces are not affected. --------- Signed-off-by: Ping Xie <pingxie@google.com>

…alkey-io#598) Make the one backwards compatible config change we are allowed to replace for removing master from our API. `masterauth` and `masteruser` are still used as an alias, but aren't explicitly referenced. As an addendum to valkey-io#591, it would be good to have this in 8. Given the related PR for updated other references for master, I just updated the ones around this specific change. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

External facing interfaces are not affected. --------- Signed-off-by: Ping Xie <pingxie@google.com>

PingXie added 7 commits June 2, 2024 18:22

Initial name changes

44e8bee

Signed-off-by: Ping Xie <pingxie@google.com>

Fix a bad renaming

af7edd3

Signed-off-by: Ping Xie <pingxie@google.com>

Fix more files

8cc25d0

Signed-off-by: Ping Xie <pingxie@google.com>

Fix more files

d70194f

Signed-off-by: Ping Xie <pingxie@google.com>

Fix clang format

bbe031f

Signed-off-by: Ping Xie <pingxie@google.com>

Update github action

a1e6fb6

Signed-off-by: Ping Xie <pingxie@google.com>

Remove the premature github action

a16c3a5

Signed-off-by: Ping Xie <pingxie@google.com>

hwware reviewed Jun 3, 2024

View reviewed changes

madolson mentioned this pull request Jun 4, 2024

Add aliases for masteruser and masterauth to primaryuser and primaryauth #598

Merged

PingXie and others added 2 commits June 7, 2024 00:29

Merge branch 'valkey-io:unstable' into names

2c3783c

Fix more names

512e7ad

Signed-off-by: Ping Xie <pingxie@google.com>

PingXie added 2 commits June 7, 2024 00:52

Merge branch 'unstable' of https://github.com/pingxie/valkey into names

c017c9b

fix more names

625d764

Signed-off-by: Ping Xie <pingxie@google.com>

hwware approved these changes Jun 7, 2024

View reviewed changes

PingXie merged commit 54c9747 into valkey-io:unstable Jun 7, 2024
18 checks passed

advaMosh pushed a commit to advaMosh/valkey that referenced this pull request Jun 10, 2024

Remove master and slave from source code (valkey-io#591)

d8163df

External facing interfaces are not affected. --------- Signed-off-by: Ping Xie <pingxie@google.com>

naglera pushed a commit to naglera/placeholderkv that referenced this pull request Jun 10, 2024

Remove master and slave from source code (valkey-io#591)

0bad102

External facing interfaces are not affected. --------- Signed-off-by: Ping Xie <pingxie@google.com>

PingXie deleted the names branch June 13, 2024 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `master` and `slave` from source code #591

Remove `master` and `slave` from source code #591

PingXie commented Jun 3, 2024

codecov bot commented Jun 3, 2024 •

edited

Loading

hwware commented Jun 3, 2024

PingXie commented Jun 3, 2024

PingXie commented Jun 3, 2024

hwware commented Jun 3, 2024

PingXie commented Jun 3, 2024

hwware commented Jun 3, 2024

hwware Jun 3, 2024

hwware Jun 3, 2024

hwware Jun 3, 2024

hwware commented Jun 7, 2024

		@@ -1073,7 +1073,8 @@ void flushAppendOnlyFile(int force) {
		* (because there's no reason, from the AOF POV, to call fsync) and then WAITAOF may wait on

		@@ -1365,9 +1366,9 @@ struct client *createAOFClient(void) {
		*/

		@@ -2408,7 +2409,7 @@ int rewriteAppendOnlyFileBackground(void) {

Remove master and slave from source code #591

Remove master and slave from source code #591

Conversation

PingXie commented Jun 3, 2024

codecov bot commented Jun 3, 2024 • edited Loading

Codecov Report

hwware commented Jun 3, 2024

PingXie commented Jun 3, 2024

PingXie commented Jun 3, 2024

hwware commented Jun 3, 2024

PingXie commented Jun 3, 2024

hwware commented Jun 3, 2024

hwware Jun 3, 2024

Choose a reason for hiding this comment

hwware Jun 3, 2024

Choose a reason for hiding this comment

hwware Jun 3, 2024

Choose a reason for hiding this comment

hwware commented Jun 7, 2024

Remove `master` and `slave` from source code #591

Remove `master` and `slave` from source code #591

codecov bot commented Jun 3, 2024 •

edited

Loading