[Bug] Incorrect cpuset role assignment in getCpuSetByRole()

[nix-oss]

Apache Cloudberry version

2.1.0-incubating

What happened

Location: src/backend/commands/resgroupcmds.c
Function getCpuSetByRole()
Lines: 1618–1649.

The getCpuSetByRole() function in src/backend/commands/resgroupcmds.c was returning cpuset values for the wrong roles.

 * Seperate cpuset by coordinator and segment
 * Return as splitcpuset
 *
 * ex:
 * cpuset = "1;4"
 * then we should assign '1' to corrdinator and '4' to segment
 * 
 * cpuset = "1"
 * assign '1' to both coordinator and segment
 */
extern char *
getCpuSetByRole(const char *cpuset)
{
	char *splitcpuset = NULL;

	if (cpuset == NULL)
	{
		ereport(ERROR,
				(errcode(ERRCODE_SYNTAX_ERROR),
				errmsg("Unexpected cpuset invalid in getCpuSetByRole")));
	}

	char *first = strchr(cpuset, ';');
	if (first == NULL)
		splitcpuset = (char *)cpuset;
	else
	{
		char *scpu = first + 1;

		/* Get result cpuset by IS_QUERY_DISPATCHER(), on master or segment */
		if (IS_QUERY_DISPATCHER())
			splitcpuset = scpu;
		else
		{
			char *mcpu = (char *)palloc0(sizeof(char) * MaxCpuSetLength);
			strncpy(mcpu, cpuset, first - cpuset);
			splitcpuset = mcpu;
		}
	}

	return splitcpuset;
}

This bug manifests in heterogeneous Cloudberry clusters where the coordinator and segment nodes have different numbers of CPU cores.
The issue becomes visible only when the core counts differ; on homogeneous clusters the swapped values would be identical and the bug would go unnoticed.

When the function receives a cpuset string formatted as "coordinator_cpus;segment_cpus" (e.g., "0-7;0-15" where coordinator has 8 cores and segments have 16 cores), it should return:

• coordinator cpuset (first part) when called from the coordinator (dispatcher),
• segment cpuset (second part) when called from segments.

However, the logic was inverted – it returned the opposite value for each role, causing incorrect CPU affinity assignment.

What you think should happen instead

Currently, the assignments are inverted. The fix is to swap the logic so that the coordinator gets mcpu and the segment gets scpu:

 * Seperate cpuset by coordinator and segment
 * Return as splitcpuset
 *
 * ex:
 * cpuset = "1;4"
 * then we should assign '1' to corrdinator and '4' to segment
 * 
 * cpuset = "1"
 * assign '1' to both coordinator and segment
 */
extern char *
getCpuSetByRole(const char *cpuset)
{
	char *splitcpuset = NULL;

	if (cpuset == NULL)
	{
		ereport(ERROR,
				(errcode(ERRCODE_SYNTAX_ERROR),
				errmsg("Unexpected cpuset invalid in getCpuSetByRole")));
	}

	char *first = strchr(cpuset, ';');
	if (first == NULL)
		splitcpuset = (char *)cpuset;
	else
	{
		char *scpu = first + 1;

		/* Get result cpuset by IS_QUERY_DISPATCHER(), on master or segment */
		if (IS_QUERY_DISPATCHER())
		{
			char *mcpu = (char *)palloc0(sizeof(char) * MaxCpuSetLength);
			strncpy(mcpu, cpuset, first - cpuset);
			splitcpuset = mcpu;
		}
		else
            splitcpuset = scpu;
	}

	return splitcpuset;
}

How to reproduce

Prerequisites:
A heterogeneous Cloudberry cluster with:

• 1 coordinator node – 8 vCPUs (cores 0–7)
• 2 segment nodes – 18 vCPUs each (cores 0–17)

Steps:

1. Connect to the coordinator node via psql.
2. Create a resource group (or alter an existing one) with a cpuset that includes both coordinator and segment CPU ranges, separated by a semicolon: ALTER RESOURCE GROUP rgroup1 SET CPUSET '4-7;9-17';
3. The command will fail with an error similar to: cpu cores %s are unavailable on the system

I'm currently trying to fix it and rebuilding the code locally to verify the changes.

I hope the details I've provided above (root cause, reproduction steps, and proposed fix) will be helpful

Operating System

Ubuntu 22.04

Anything else

No response

Are you willing to submit PR?

• Yes, I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct.

[github-actions]

Hi, @nix-oss welcome!🎊 Thanks for taking the time to point this out.🙌

[my-ship-it]

Hi @nix-oss, thank you so much for this excellent report! 🙌 The root cause analysis, the reproduction steps, and the proposed fix are all spot-on — this is one of the nicest bug reports we've received. I've verified it against the current main branch and can confirm everything you described.

A bit of history to back you up: I traced this with git log -L, and the inverted logic came in with commit 0080be29610 ("Simplify RG cpuset seperated by coordinator/segment (#14637)", Dec 2022), which refactored the original strtok-based implementation. The pre-refactor code actually had it right:

/* Get result cpuset by gprole, on master or segment */
if (Gp_role == GP_ROLE_EXECUTE && arraycpuset[1] != NULL)
    splitcpuset = arraycpuset[1];   /* segment gets the second part */
else
    splitcpuset = arraycpuset[0];   /* coordinator gets the first part */

The two branches simply got swapped during the rewrite. Funnily enough, the variable names still tell the true story: the dispatcher branch returns scpu (segment cpu) and the segment branch builds mcpu (master cpu) — so your reading of the intent is exactly right. Upstream GPDB 7 picked up the same code from the same PR, so it inherits this bug too.

One thing worth adding to your impact analysis: on homogeneous clusters the bug doesn't just go unnoticed — the DDL succeeds and the affinity is silently applied to the wrong roles (coordinator gets the segment cores and vice versa). So the fix helps everyone, not only heterogeneous deployments. Your heterogeneous setup is what finally made it visible — great catch!

A small suggestion if you send a PR: it might be worth making both branches return a palloc'd copy, rather than returning first + 1 (a pointer into the caller's string) on one path. There's a call site in InitResGroups (resgroup.c, the fallback-core path) that does snprintf(cpuset2, MaxCpuSetLength, ...) on the returned pointer, which writes through into the caller's caps.cpuset buffer. That hazard exists both before and after the swap, so it'd be lovely to clean it up in the same patch. A small unit test on getCpuSetByRole() would also help guard against re-regression, since single-host test setups can't tell the two roles' cpusets apart.

Please do feel encouraged to open that PR — you've already done the hard part, and we'd be glad to review it. Thanks again for the careful investigation and for making Cloudberry better! 🚀

[nix-oss]

Hi, @my-ship-it!

Thank you for the detailed response.
I appreciate the confirmation and the additional context regarding the commit history.
Special thanks for the clarification regarding the palloc copy and unit test — this is very important information.

I would be glad to prepare a PR.