code review from madelyn

Signed-off-by: Binbin <[email protected]>

Author: enjoy-binbin

src/cluster_migrateslots.c

@@ -76,6 +76,7 @@ typedef struct slotMigrationJob {
                                                * cleanup is done. */
     sds description;                          /* Description, used for
                                                * logging. */
+    size_t stat_cow_bytes;                    /* Copy on write bytes during slot migration fork. */
 
     /* State needed during client establishment */
     connection *conn; /* Connection to slot import source node. */
@@ -1297,7 +1298,7 @@ int slotExportJobBeginSnapshotToTargetSocket(slotMigrationJob *job) {
         close(server.rdb_pipe_read);
 
         serverSetProcTitle("valkey-slot-migration-to-target");
-        serverSetCpuAffinity(server.slot_migration_cpulist);
+        serverSetCpuAffinity(server.bgsave_cpulist);
 
         int retval = childSnapshotForSyncSlot(&aof, job);
         if (retval == C_OK && rioFlush(&aof) == 0) retval = C_ERR;
@@ -1366,6 +1367,7 @@ void backgroundSlotMigrationDoneHandler(int exitcode, int bysignal) {
         }
         if (!bysignal && exitcode == 0) {
             slotExportBeginStreaming(job);
+            job->stat_cow_bytes = server.stat_slot_migration_cow_bytes;
         } else {
             serverLog(LL_WARNING,
                       "Child process failed to snapshot slot migration %s",
@@ -2043,6 +2045,8 @@ void clusterCommandGetSlotMigrations(client *c) {
         addReplyBulkCString(c, slotMigrationJobStateToString(job->state));
         addReplyBulkCString(c, "message");
         addReplyBulkCString(c, job->status_msg ? job->status_msg : "");
+        addReplyBulkCString(c, "last_cow_size");
+        addReplyLongLong(c, (long long)job->stat_cow_bytes);
     }
 }
 

src/commands/cluster-getslotmigrations.json

@@ -40,7 +40,9 @@
                         "pattern": "^([0-9]+-[0-9]+)( [0-9]+-[0-9]+)*$"
                     },
                     "node": {
-                        "type": "string"
+                        "description": "The source node name in an import job or the target node name in an export job.",
+                        "type": "string",
+                        "pattern": "^[0-9a-fA-F]{40}$"
                     },
                     "create_time": {
                         "description": "Creation time, in seconds since the unix epoch.",
@@ -55,10 +57,16 @@
                         "type": "integer"
                     },
                     "state": {
+                        "description": "Human readable string representing the migration job state.",
                         "type": "string"
                     },
                     "message": {
+                        "description": "Human readable status message with more details.",
                         "type": "string"
+                    },
+                    "last_cow_size": {
+                        "description": "Copy on write bytes during slot migration fork.",
+                        "type": "integer"
                     }
                 }
             }

src/config.c

@@ -3252,7 +3252,6 @@ standardConfig static_configs[] = {
     createStringConfig("bio-cpulist", "bio_cpulist", IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bio_cpulist, NULL, NULL, NULL),
     createStringConfig("aof-rewrite-cpulist", "aof_rewrite_cpulist", IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.aof_rewrite_cpulist, NULL, NULL, NULL),
     createStringConfig("bgsave-cpulist", "bgsave_cpulist", IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bgsave_cpulist, NULL, NULL, NULL),
-    createStringConfig("slot-migration-cpulist", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.slot_migration_cpulist, NULL, NULL, NULL),
     createStringConfig("ignore-warnings", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.ignore_warnings, "", NULL, NULL),
     createStringConfig("proc-title-template", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.proc_title_template, CONFIG_DEFAULT_PROC_TITLE_TEMPLATE, isValidProcTitleTemplate, updateProcTitleTemplate),
     createStringConfig("bind-source-addr", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bind_source_addr, NULL, NULL, NULL),

src/server.c

@@ -1391,6 +1391,7 @@ void checkChildrenDone(void) {
                       "child_type: %s, child_pid = %d",
                       strerror(errno), strChildType(server.child_type), (int)server.child_pid);
         } else if (pid == server.child_pid) {
+            if (!bysignal && exitcode == 0) receiveChildInfo();
             if (server.child_type == CHILD_TYPE_RDB) {
                 backgroundSaveDoneHandler(exitcode, bysignal);
             } else if (server.child_type == CHILD_TYPE_AOF) {
@@ -1403,7 +1404,6 @@ void checkChildrenDone(void) {
                 serverPanic("Unknown child type %d for child pid %d", server.child_type, server.child_pid);
                 exit(1);
             }
-            if (!bysignal && exitcode == 0) receiveChildInfo();
             resetChildState();
         } else {
             if (!ldbRemoveChild(pid)) {
@@ -6041,8 +6041,7 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) {
                 "aof_last_cow_size:%zu\r\n", server.stat_aof_cow_bytes,
                 "module_fork_in_progress:%d\r\n", server.child_type == CHILD_TYPE_MODULE,
                 "module_fork_last_cow_size:%zu\r\n", server.stat_module_cow_bytes,
-                "slot_migration_fork_in_progress:%d\r\n", server.child_type == CHILD_TYPE_SLOT_MIGRATION,
-                "slot_migration_fork_last_cow_size:%zu\r\n", server.stat_slot_migration_cow_bytes));
+                "slot_migration_fork_in_progress:%d\r\n", server.child_type == CHILD_TYPE_SLOT_MIGRATION));
 
         if (server.aof_enabled) {
             info = sdscatprintf(

tests/unit/introspection.tcl

@@ -1209,7 +1209,6 @@ start_server {tags {"introspection"}} {
             bio-cpulist
             aof-rewrite-cpulist
             bgsave-cpulist
-            slot-migration-cpulist
             server_cpulist
             bio_cpulist
             aof_rewrite_cpulist

valkey.conf

@@ -2602,11 +2602,8 @@ jemalloc-bg-thread yes
 # Set aof rewrite child process to cpu affinity 8,9,10,11:
 # aof-rewrite-cpulist 8-11
 #
-# Set bgsave child process to cpu affinity 1,10,11:
+# Set bgsave (or slot migration) child process to cpu affinity 1,10,11:
 # bgsave-cpulist 1,10-11
-#
-# Set slot migration child process to cpu affinity 1,8,9,10:
-# slot-migration-cpulist 1,8-10
 
 # In some cases the server will emit warnings and even refuse to start if it detects
 # that the system is in bad state, it is possible to suppress these warnings