Skip to content

Commit 732832c

Browse files
committed
Fix inverted cpuset assignment in getCpuSetByRole()
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 #1862
1 parent 0764227 commit 732832c

3 files changed

Lines changed: 146 additions & 4 deletions

File tree

src/backend/commands/resgroupcmds.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,17 +1632,21 @@ getCpuSetByRole(const char *cpuset)
16321632
splitcpuset = (char *)cpuset;
16331633
else
16341634
{
1635-
char *scpu = first + 1;
1635+
char *second = first + 1;
16361636

16371637
/* Get result cpuset by IS_QUERY_DISPATCHER(), on master or segment */
16381638
if (IS_QUERY_DISPATCHER())
1639-
splitcpuset = scpu;
1640-
else
16411639
{
16421640
char *mcpu = (char *)palloc0(sizeof(char) * MaxCpuSetLength);
16431641
strncpy(mcpu, cpuset, first - cpuset);
16441642
splitcpuset = mcpu;
16451643
}
1644+
else
1645+
{
1646+
char *scpu = (char *)palloc0(sizeof(char) * MaxCpuSetLength);
1647+
strlcpy(scpu, second, MaxCpuSetLength);
1648+
splitcpuset = scpu;
1649+
}
16461650
}
16471651

16481652
return splitcpuset;

src/backend/commands/test/Makefile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ subdir=src/backend/commands
22
top_builddir=../../../..
33
include $(top_builddir)/src/Makefile.global
44

5-
TARGETS=tablecmds
5+
TARGETS=tablecmds resgroupcmds
66

77
include $(top_srcdir)/src/backend/mock.mk
88

99
tablecmds.t: \
1010
$(MOCK_DIR)/backend/access/aocs/aocs_compaction_mock.o \
1111
$(MOCK_DIR)/backend/access/hash/hash_mock.o \
1212
$(MOCK_DIR)/backend/utils/fmgr/fmgr_mock.o
13+
14+
resgroupcmds.t: \
15+
$(MOCK_DIR)/backend/utils/fmgr/fmgr_mock.o
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
*
20+
* resgroupcmds_test.c
21+
*
22+
* IDENTIFICATION
23+
* src/backend/commands/test/resgroupcmds_test.c
24+
*
25+
*-------------------------------------------------------------------------
26+
*/
27+
28+
#include <stdarg.h>
29+
#include <stddef.h>
30+
#include <setjmp.h>
31+
#include "cmockery.h"
32+
33+
#include "../resgroupcmds.c"
34+
35+
/*
36+
* Helper: simulate running on the coordinator (dispatcher).
37+
*/
38+
static void
39+
set_role_coordinator(void)
40+
{
41+
GpIdentity.segindex = MASTER_CONTENT_ID;
42+
}
43+
44+
/*
45+
* Helper: simulate running on a segment.
46+
*/
47+
static void
48+
set_role_segment(void)
49+
{
50+
GpIdentity.segindex = 0;
51+
}
52+
53+
/*
54+
* NULL input must raise an ERROR.
55+
*/
56+
static void
57+
test__getCpuSetByRole_null_input(void **state)
58+
{
59+
PG_TRY();
60+
{
61+
getCpuSetByRole(NULL);
62+
fail_msg("expected ereport(ERROR) for NULL cpuset");
63+
}
64+
PG_CATCH();
65+
{
66+
FlushErrorState();
67+
}
68+
PG_END_TRY();
69+
}
70+
71+
/*
72+
* cpuset without a separator: same value must be returned
73+
* regardless of the role.
74+
*/
75+
static void
76+
test__getCpuSetByRole_no_separator(void **state)
77+
{
78+
const char *input = "0-7";
79+
char *result;
80+
81+
set_role_coordinator();
82+
result = getCpuSetByRole(input);
83+
assert_string_equal(result, "0-7");
84+
85+
set_role_segment();
86+
result = getCpuSetByRole(input);
87+
assert_string_equal(result, "0-7");
88+
}
89+
90+
/*
91+
* cpuset with a separator, called as coordinator:
92+
* must return the part BEFORE the ';'.
93+
*/
94+
static void
95+
test__getCpuSetByRole_with_separator_as_coordinator(void **state)
96+
{
97+
const char *input = "0-7;0-15";
98+
char *result;
99+
100+
set_role_coordinator();
101+
result = getCpuSetByRole(input);
102+
assert_string_equal(result, "0-7");
103+
}
104+
105+
/*
106+
* cpuset with a separator, called as segment:
107+
* must return the part AFTER the ';'.
108+
*/
109+
static void
110+
test__getCpuSetByRole_with_separator_as_segment(void **state)
111+
{
112+
const char *input = "0-7;0-15";
113+
char *result;
114+
115+
set_role_segment();
116+
result = getCpuSetByRole(input);
117+
assert_string_equal(result, "0-15");
118+
}
119+
120+
int
121+
main(int argc, char *argv[])
122+
{
123+
cmockery_parse_arguments(argc, argv);
124+
125+
const UnitTest tests[] = {
126+
unit_test(test__getCpuSetByRole_null_input),
127+
unit_test(test__getCpuSetByRole_no_separator),
128+
unit_test(test__getCpuSetByRole_with_separator_as_coordinator),
129+
unit_test(test__getCpuSetByRole_with_separator_as_segment)
130+
};
131+
132+
MemoryContextInit();
133+
134+
return run_tests(tests);
135+
}

0 commit comments

Comments
 (0)