DAOS-11955 pool: Ensure a PS is inside pool (#13046) (#14448)

It was found that a PS leader may enter ds_pool_plan_svc_reconfs with
itself being an undesirable replica. This may lead to an assertion
failure at "move n replicas from undesired to to_remove" in
ds_pool_plan_svc_reconfs. Moreover, such a PS leader may be outside of
the pool group, making it incapable of performing many duties that
involve collective communication.

This patch therefore ensures that a PS leader will remove undesirable PS
replicas synchronously before committing a pool map modification that
introduces new undesirable PS replicas. (If we were to keep an
undesirable PS replica, it might become a PS leader.)

  - Extend and clean up pool_svc_sched.
      * Allow pool_svc_reconf_ult to return an error, so that we can
	fail a pool map modification if its synchronous PS replica
        removal fails.
      * Allow pool_svc_reconf_ult to get an argument, so that we can
	tell pool_svc_reconf_ult whether we want a synchronous
        remove-only run or an asyncrhonous add-remove run.
      * Move pool_svc_sched.{psc_svc_rf,psc_force_notify} up to
	pool_svc.
  - Prevent pool_svc_step_up_cb from canceling in-progress
    reconfigurations by comparing pool map versions for which the
    reconfigurations are scheduled.
  - Rename POOL_GROUP_MAP_STATUS to POOL_GROUP_MAP_STATES so that we are
    consistent with the pool_map module.

Signed-off-by: Li Wei <[email protected]>

Also backported this one:
DAOS-14730 pool: Clean up map update logging (#13709)

This patch cleans up the pool map update logging on the client side and
the engine side. A few notable changes:

  - In dc_pool_map_update, if the incoming map is of the same version as
    the one we already have, do not perform the update.

Signed-off-by: Li Wei <[email protected]>

src/pool/cli.c

@@ -471,32 +471,23 @@ int
 dc_pool_map_update(struct dc_pool *pool, struct pool_map *map, bool connect)
 {
 	unsigned int	map_version;
+	unsigned int	map_version_before = 0;
 	int		rc;
 
 	D_ASSERT(map != NULL);
 	map_version = pool_map_get_version(map);
 
-	if (pool->dp_map == NULL) {
-		rc = pl_map_update(pool->dp_pool, map, connect, DEFAULT_PL_TYPE);
-		if (rc != 0)
-			D_GOTO(out, rc);
-
-		D_DEBUG(DB_MD, DF_UUID": init pool map: %u\n",
-			DP_UUID(pool->dp_pool), pool_map_get_version(map));
-		D_GOTO(out_update, rc = 0);
-	}
+	if (pool->dp_map != NULL)
+		map_version_before = pool_map_get_version(pool->dp_map);
 
-	if (map_version < pool_map_get_version(pool->dp_map)) {
-		D_DEBUG(DB_MD, DF_UUID": got older pool map: %u -> %u %p\n",
-			DP_UUID(pool->dp_pool),
-			pool_map_get_version(pool->dp_map), map_version, pool);
+	if (map_version <= map_version_before) {
+		D_DEBUG(DB_MD, DF_UUID ": ignored pool map update: version=%u->%u pool=%p\n",
+			DP_UUID(pool->dp_pool), map_version_before, map_version, pool);
 		D_GOTO(out, rc = 0);
 	}
 
-	D_DEBUG(DB_MD, DF_UUID": updating pool map: %u -> %u\n",
-		DP_UUID(pool->dp_pool),
-		pool->dp_map == NULL ?
-		0 : pool_map_get_version(pool->dp_map), map_version);
+	D_DEBUG(DB_MD, DF_UUID ": updating pool map: version=%u->%u\n", DP_UUID(pool->dp_pool),
+		map_version_before, map_version);
 
 	rc = pl_map_update(pool->dp_pool, map, connect, DEFAULT_PL_TYPE);
 	if (rc != 0) {
@@ -505,12 +496,14 @@ dc_pool_map_update(struct dc_pool *pool, struct pool_map *map, bool connect)
 		D_GOTO(out, rc);
 	}
 
-	pool_map_decref(pool->dp_map);
-out_update:
+	if (pool->dp_map != NULL)
+		pool_map_decref(pool->dp_map);
 	pool_map_addref(map);
 	pool->dp_map = map;
 	if (pool->dp_map_version_known < map_version)
 		pool->dp_map_version_known = map_version;
+	D_INFO(DF_UUID ": updated pool map: version=%u->%u\n", DP_UUID(pool->dp_pool),
+	       map_version_before, map_version);
 out:
 	return rc;
 }

src/pool/srv_internal.h

@@ -16,8 +16,17 @@
 #include <daos_security.h>
 #include <gurt/telemetry_common.h>
 
-/* Map status of ranks that make up the pool group */
-#define POOL_GROUP_MAP_STATUS (PO_COMP_ST_UP | PO_COMP_ST_UPIN | PO_COMP_ST_DRAIN)
+/* Map states of ranks that make up the pool group */
+#define POOL_GROUP_MAP_STATES (PO_COMP_ST_UP | PO_COMP_ST_UPIN | PO_COMP_ST_DRAIN)
+
+/* Map states of ranks that make up the pool service */
+#define POOL_SVC_MAP_STATES (PO_COMP_ST_UP | PO_COMP_ST_UPIN)
+
+/*
+ * Since we want all PS replicas to belong to the pool group,
+ * POOL_SVC_MAP_STATES must be a subset of POOL_GROUP_MAP_STATES.
+ */
+D_CASSERT((POOL_SVC_MAP_STATES & POOL_GROUP_MAP_STATES) == POOL_SVC_MAP_STATES);
 
 /**
  * Global pool metrics