Update cpuset error message to include more helpful hints (#5124)

src/backend/commands/resgroupcmds.c

@@ -239,24 +239,16 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt)
 		}
 		else if (!CpusetIsEmpty(caps.cpuset))
 		{
-			if (gp_resource_group_enable_cgroup_cpuset)
-			{
-				ResGroupOps_SetCpuSet(groupid, caps.cpuset);
-				/* reset default group, subtract new group cpu cores */
-				char defaultGroupCpuset[MaxCpuSetLength];
-				ResGroupOps_GetCpuSet(DEFAULT_CPUSET_GROUP_ID,
-									  defaultGroupCpuset,
-									  MaxCpuSetLength);
-				CpusetDifference(defaultGroupCpuset, caps.cpuset, MaxCpuSetLength);
-				ResGroupOps_SetCpuSet(DEFAULT_CPUSET_GROUP_ID, defaultGroupCpuset);
-			}
-			else
-			{
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("cpuset is disabled for cpuset/gpdb "
-								"does not exist")));
-			}
+			EnsureCpusetIsAvailable(ERROR);
+
+			ResGroupOps_SetCpuSet(groupid, caps.cpuset);
+			/* reset default group, subtract new group cpu cores */
+			char defaultGroupCpuset[MaxCpuSetLength];
+			ResGroupOps_GetCpuSet(DEFAULT_CPUSET_GROUP_ID,
+								  defaultGroupCpuset,
+								  MaxCpuSetLength);
+			CpusetDifference(defaultGroupCpuset, caps.cpuset, MaxCpuSetLength);
+			ResGroupOps_SetCpuSet(DEFAULT_CPUSET_GROUP_ID, defaultGroupCpuset);
 		}
 		SIMPLE_FAULT_INJECTOR(CreateResourceGroupFail);
 	}
@@ -273,7 +265,6 @@ void
 DropResourceGroup(DropResourceGroupStmt *stmt)
 {
 	Relation	 pg_resgroup_rel;
-	Relation	 pg_resgroupcapability_rel;
 	HeapTuple	 tuple;
 	ScanKeyData	 scankey;
 	SysScanDesc	 sscan;
@@ -402,19 +393,8 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
 	}
 	else if (limitType == RESGROUP_LIMIT_TYPE_CPUSET)
 	{
-		if (!IsResGroupActivated())
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("must specify cpuset when resource group is activated")));
-		}
-		else if (!gp_resource_group_enable_cgroup_cpuset)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("cpuset is disabled for cpuset/gpdb "
-							"does not exist")));
-		}
+		EnsureCpusetIsAvailable(ERROR);
+
 		cpuset = defGetString(defel);
 		checkCpusetSyntax(cpuset);
 	}
@@ -777,7 +757,7 @@ GetResGroupNameForId(Oid oid)
 		name = pstrdup(NameStr(*resGroupName));
 	}
 	else
-		return "unknow";
+		return "unknown";
 
 	ReleaseSysCache(tuple);
 
@@ -1072,11 +1052,8 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps)
 		}
 	}
 
-	if ((mask & (1 << RESGROUP_LIMIT_TYPE_CPUSET)) &&
-		!IsResGroupActivated())
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				errmsg("must specify cpuset when resource group is activated")));
+	if ((mask & (1 << RESGROUP_LIMIT_TYPE_CPUSET)))
+		EnsureCpusetIsAvailable(ERROR);
 
 	if (!(mask & (1 << RESGROUP_LIMIT_TYPE_MEMORY)))
 		ereport(ERROR,
@@ -1301,15 +1278,13 @@ validateCapabilities(Relation rel,
 	int totalCpu = caps->cpuRateLimit;
 	int totalMem = caps->memLimit;
 	char cpusetAll[MaxCpuSetLength] = {0};
-	Bitmapset *bmsUnused = NULL;
+	char cpusetMissing[MaxCpuSetLength] = {0};
 	Bitmapset *bmsCurrent = NULL;
+	Bitmapset *bmsCommon = NULL;
 
-	if (!gp_resource_group_enable_cgroup_cpuset &&
-		!CpusetIsEmpty(caps->cpuset))
+	if (!CpusetIsEmpty(caps->cpuset))
 	{
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cpuset is disabled for cpuset/gpdb does not exist")));
+		EnsureCpusetIsAvailable(ERROR);
 	}
 
 	/*
@@ -1318,22 +1293,31 @@ validateCapabilities(Relation rel,
 	if (IsResGroupActivated() &&
 		gp_resource_group_enable_cgroup_cpuset)
 	{
+		Bitmapset *bmsAll = NULL;
+
 		/* Get all available cores */
 		ResGroupOps_GetCpuSet(RESGROUP_ROOT_ID,
 							  cpusetAll,
 							  MaxCpuSetLength);
-		bmsUnused = CpusetToBitset(cpusetAll, MaxCpuSetLength);
+		bmsAll = CpusetToBitset(cpusetAll, MaxCpuSetLength);
 		/* Check whether the cores in this group are available */
 		if (!CpusetIsEmpty(caps->cpuset))
 		{
+			Bitmapset *bmsMissing = NULL;
+
 			bmsCurrent = CpusetToBitset(caps->cpuset, MaxCpuSetLength);
-			if (!bms_is_subset(bmsCurrent, bmsUnused))
+			bmsCommon = bms_intersect(bmsCurrent, bmsAll);
+			bmsMissing = bms_difference(bmsCurrent, bmsCommon);
+
+			if (!bms_is_empty(bmsMissing))
 			{
+				BitsetToCpuset(bmsMissing, cpusetMissing, MaxCpuSetLength);
+
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						errmsg("some cores of cpuset are unavailable")));
+						 errmsg("cpu cores %s are unavailable on the system",
+								cpusetMissing)));
 			}
-			bmsUnused = bms_del_members(bmsUnused, bmsCurrent);
 		}
 	}
 
@@ -1402,28 +1386,29 @@ validateCapabilities(Relation rel,
 			/*
 			 * do the check when resource group is activated
 			 */
-			if (IsResGroupActivated())
+			if (IsResGroupActivated() && !CpusetIsEmpty(caps->cpuset))
 			{
 				proposedStr = TextDatumGetCString(proposedDatum);
 				if (!CpusetIsEmpty(proposedStr))
 				{
-					if (gp_resource_group_enable_cgroup_cpuset)
-					{
-						bmsCurrent = CpusetToBitset(proposedStr, MaxCpuSetLength);
-						if (!bms_is_subset(bmsCurrent, bmsUnused))
-						{
-							ereport(ERROR,
-									(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-									errmsg("some cores of cpuset are unavailable")));
-						}
-						bmsUnused = bms_del_members(bmsUnused, bmsCurrent);
-					}
-					else
+					Bitmapset *bmsOther = NULL;
+
+					EnsureCpusetIsAvailable(ERROR);
+
+					Assert(!bms_is_empty(bmsCurrent));
+
+					bmsOther = CpusetToBitset(proposedStr, MaxCpuSetLength);
+					bmsCommon = bms_intersect(bmsCurrent, bmsOther);
+
+					if (!bms_is_empty(bmsCommon))
 					{
+						BitsetToCpuset(bmsCommon, cpusetMissing, MaxCpuSetLength);
+
 						ereport(ERROR,
 								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("cpuset is disabled for cpuset/gpdb "
-										"does not exist")));
+								 errmsg("cpu cores %s are used by resource group %s",
+										cpusetMissing,
+										GetResGroupNameForId(resgroupid))));
 					}
 				}
 			}

src/backend/utils/resgroup/resgroup.c

@@ -588,47 +588,55 @@ InitResGroups(void)
 		}
 		else
 		{
-			if (gp_resource_group_enable_cgroup_cpuset)
+			Bitmapset *bmsCurrent = CpusetToBitset(caps.cpuset,
+												   MaxCpuSetLength);
+			Bitmapset *bmsCommon = bms_intersect(bmsCurrent, bmsUnused);
+			Bitmapset *bmsMissing = bms_difference(bmsCurrent, bmsCommon);
+
+			/*
+			 * Do not call EnsureCpusetIsAvailable() here as resource group is
+			 * not activated yet
+			 */
+			if (!gp_resource_group_enable_cgroup_cpuset)
 			{
-				bool allCoreAvailable = true;
-				Bitmapset *bmsCurrent = CpusetToBitset(caps.cpuset,
-													   MaxCpuSetLength);
+				ereport(WARNING,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("cgroup is not properly configured to use the cpuset feature"),
+						 errhint("Extra cgroup configurations are required to enable this feature, "
+								 "please refer to the Greenplum Documentations for details")));
+			}
 
-				Assert(caps.cpuRateLimit == CPU_RATE_LIMIT_DISABLED);
+			Assert(caps.cpuRateLimit == CPU_RATE_LIMIT_DISABLED);
 
-				allCoreAvailable = bms_is_subset(bmsCurrent, bmsUnused);
-				if (allCoreAvailable)
-				{
-					/*
-					 * write cpus to corresponding file
-					 * if all the cores are available
-					 */
-					ResGroupOps_SetCpuSet(groupId, caps.cpuset);
-					bmsUnused = bms_del_members(bmsUnused, bmsCurrent);
-				}
-				else
-				{
-					/*
-					 * if some of the cores are unavailable, just set defaultCore
-					 * to this group and send a warning message, so the system
-					 * can startup, then DBA can fix it
-					 */
-					snprintf(cpuset, MaxCpuSetLength, "%d", defaultCore);
-					ResGroupOps_SetCpuSet(groupId, cpuset);
-					ereport(WARNING,
-							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("some of the cpu cores are unavailable "
-									"in group(%d), using core(%d) instead",
-									groupId,
-									defaultCore)));
-				}
+			if (bms_is_empty(bmsMissing))
+			{
+				/*
+				 * write cpus to corresponding file
+				 * if all the cores are available
+				 */
+				ResGroupOps_SetCpuSet(groupId, caps.cpuset);
+				bmsUnused = bms_del_members(bmsUnused, bmsCurrent);
 			}
 			else
 			{
+				char		cpusetMissing[MaxCpuSetLength] = {0};
+
+				/*
+				 * if some of the cores are unavailable, just set defaultCore
+				 * to this group and send a warning message, so the system
+				 * can startup, then DBA can fix it
+				 */
+				snprintf(cpuset, MaxCpuSetLength, "%d", defaultCore);
+				ResGroupOps_SetCpuSet(groupId, cpuset);
+				BitsetToCpuset(bmsMissing, cpusetMissing, MaxCpuSetLength);
 				ereport(WARNING,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("cpuset is disabled for cpuset/gpdb "
-								"does not exist")));
+						 errmsg("cpu cores %s are unavailable on the system "
+								"in resource group %s",
+								cpusetMissing, GetResGroupNameForId(groupId)),
+						 errhint("using core %d for this resource group, "
+								 "please adjust the settings and restart",
+								 defaultCore)));
 			}
 		}
 
@@ -751,13 +759,6 @@ ResGroupDropFinish(const ResourceGroupCallbackContext *callbackCtx,
 					CpusetUnion(cpuset, group->caps.cpuset, MaxCpuSetLength);
 					ResGroupOps_SetCpuSet(DEFAULT_CPUSET_GROUP_ID, cpuset);
 				}
-				else
-				{
-					ereport(WARNING,
-							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("cpuset is disabled for cpuset/gpdb "
-									"does not exist")));
-				}
 			}
 
 			ResGroupOps_DestroyGroup(callbackCtx->groupid, migrate);
@@ -3959,3 +3960,32 @@ CpusetDifference(char *cpuset1, const char *cpuset2, int len)
 {
 	cpusetOperation(cpuset1, cpuset2, len, true);
 }
+
+/*
+ * ensure that cpuset is available.
+ */
+bool
+EnsureCpusetIsAvailable(int elevel)
+{
+	if (!IsResGroupActivated())
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("resource group must be enabled to use cpuset feature")));
+
+		return false;
+	}
+
+	if (!gp_resource_group_enable_cgroup_cpuset)
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("cgroup is not properly configured to use the cpuset feature"),
+				 errhint("Extra cgroup configurations are required to enable this feature, "
+						 "please refer to the Greenplum Documentations for details")));
+
+		return false;
+	}
+
+	return true;
+}

src/include/utils/resgroup.h

@@ -183,6 +183,7 @@ extern void CpusetUnion(char *cpuset1, const char *cpuset2, int len);
 extern void CpusetDifference(char *cpuset1, const char *cpuset2, int len);
 extern bool CpusetIsEmpty(const char *cpuset);
 extern void SetCpusetEmpty(char *cpuset, int cpusetSize);
+extern bool EnsureCpusetIsAvailable(int elevel);
 
 #define LOG_RESGROUP_DEBUG(...) \
 	do {if (Debug_resource_group) elog(__VA_ARGS__); } while(false);

src/test/isolation2/expected/resgroup/resgroup_syntax.out

@@ -144,7 +144,7 @@ CREATE RESOURCE GROUP rg_test_group WITH (cpuset=' 0 ', memory_limit=5);
 ERROR:  cpuset invalid
 ---- suppose the core numbered 1024 is not exist
 CREATE RESOURCE GROUP rg_test_group WITH (cpuset='1024', memory_limit=5);
-ERROR:  some cores of cpuset are unavailable
+ERROR:  cpu cores 1024 are unavailable on the system
 CREATE RESOURCE GROUP rg_test_group WITH (cpuset='0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,', memory_limit=5);
 ERROR:  the length of cpuset reached the upper limit 1024
 -- can't alter to invalid cpuset
@@ -170,7 +170,7 @@ ALTER RESOURCE GROUP rg_test_group set CPUSET ' 0 ';
 ERROR:  cpuset invalid
 ---- suppose the core numbered 1024 is not exist
 ALTER RESOURCE GROUP rg_test_group set CPUSET '1024';
-ERROR:  some cores of cpuset are unavailable
+ERROR:  cpu cores 1024 are unavailable on the system
 ALTER RESOURCE GROUP rg_test_group set CPUSET '0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,';
 ERROR:  the length of cpuset reached the upper limit 1024
 DROP RESOURCE GROUP rg_test_group;
@@ -249,7 +249,7 @@ ERROR:  resource group concurrency must be 0 when group memory_auditor is cgroup
 CREATE RESOURCE GROUP rg_test_group1 WITH (cpuset='0', memory_limit=10);
 CREATE
 CREATE RESOURCE GROUP rg_test_group2 WITH (cpuset='0', memory_limit=10);
-ERROR:  some cores of cpuset are unavailable
+ERROR:  cpu cores 0 are used by resource group rg_test_group1
 DROP RESOURCE GROUP rg_test_group1;
 DROP
 

src/test/regress/expected/resource_group_cpuset.out

@@ -2,10 +2,10 @@
 -- Test: cpuset cannot be specified when group is disabled.
 --
 CREATE RESOURCE GROUP resource_group1 WITH (memory_limit=5, cpuset='0');
-ERROR:  must specify cpuset when resource group is activated
+ERROR:  resource group must be enabled to use cpuset feature
 CREATE RESOURCE GROUP resource_group1 WITH (memory_limit=5, cpu_rate_limit=5);
 WARNING:  resource group is disabled
 HINT:  To enable set gp_resource_manager=group
 ALTER RESOURCE GROUP resource_group1 SET cpuset '0';
-ERROR:  must specify cpuset when resource group is activated
+ERROR:  resource group must be enabled to use cpuset feature
 DROP RESOURCE GROUP resource_group1;