Fix inverted cpuset assignment - #1863
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi, @nix-oss welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!
e658827 to
22a76dc
Compare
|
LGTM, but it's better to squash to one commit and add more commit message. What's more, this change may influence compatibility for greenplum or older cloudberry versions which maybe should be token into consideration. |
Previously, the function returned the wrong cpuset for coordinator and segment roles when the cpuset string contained a semicolon separator (e.g., "0-7;0-15"). - Coordinator now receives the first part (before ';') - Segment now receives the second part (after ';') - Added unit tests covering both branches with different values - Added Apache license header to the new test file Fixes apache#1862
22a76dc to
732832c
Compare
|
Hi @zhangwenchao-123. Regarding compatibility — this is indeed an important point. It seems this CPU management mechanism is either not heavily used, or limited to homogeneous environments, which is why this issue went unnoticed for so long. |
Thanks for your contribution, LGTM. |
3 participants
Fixes #1862
What does this PR do?
Fix inverted cpuset assignment in
getCpuSetByRole(). The function returned the wrong cpuset for coordinator and segment roles when the cpuset string contains a semicolon separator (e.g., "0-7;0-15"). This caused incorrect CPU affinity settings on clusters.Additionally, this PR addresses the suggestion from the @my-ship-it to return a palloc'd copy instead of a pointer into the caller's string.
A unit test for
getCpuSetByRole()is also added to guard against future regressions.Type of Change
Test Plan
Impact
Performance:
This fix ensures that CPU cores are assigned to the coordinator and segments exactly as configured by the administrator. It prevents unintended usage of cores that may be reserved for other critical system tasks.
User-facing changes:
Users can now successfully set cpuset strings with a semicolon separator (e.g.,
ALTER RESOURCE GROUP ... SET CPUSET '4-7;9-17') without receiving a "cpu cores unavailable" error.Checklist