Skip to content

Commit 18fd896

Browse files
committed
fix namegen algo and add more test cases covering corner cases
1 parent 95c5d16 commit 18fd896

File tree

2 files changed

+93
-12
lines changed

2 files changed

+93
-12
lines changed

internal/xds/balancer/clusterresolver/configbuilder_childname.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,16 @@ func (ng *nameGenerator) generate(priorities [][]xdsresource.Locality) []string
6666
for _, priority := range priorities {
6767
var nameFound string
6868
for _, locality := range priority {
69-
if name, ok := ng.currentNames[locality.ID]; ok {
70-
if !usedNames[name] {
71-
nameFound = name
72-
break
73-
}
69+
if name, ok := ng.currentNames[locality.ID]; ok && !usedNames[name] {
70+
nameFound = name
71+
break
7472
}
7573
}
7674

77-
for _, locality := range priority {
78-
if name, ok := ng.firstAssignedNames[locality.ID]; ok {
79-
if !usedNames[name] {
75+
if nameFound == "" {
76+
// If no name found in current names, check first assigned names instead of generating completely new names.
77+
for _, locality := range priority {
78+
if name, ok := ng.firstAssignedNames[locality.ID]; ok && !usedNames[name] {
8079
nameFound = name
8180
break
8281
}
@@ -95,11 +94,12 @@ func (ng *nameGenerator) generate(priorities [][]xdsresource.Locality) []string
9594
}
9695
}
9796
ret = append(ret, nameFound)
98-
usedNames[nameFound] = true
99-
97+
// All localities in this priority share the same name. Add them all to
98+
// the new map.
10099
for _, locality := range priority {
101100
newNames[locality.ID] = nameFound
102101
}
102+
usedNames[nameFound] = true
103103
}
104104
ng.currentNames = newNames
105105
return ret

internal/xds/balancer/clusterresolver/configbuilder_childname_test.go

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,35 @@ func Test_nameGenerator_generate(t *testing.T) {
105105
},
106106
want: []string{"priority-0-0", "priority-0-2", "priority-0-1"},
107107
},
108+
{
109+
name: "split and reverse priority",
110+
inputs: [][][]xdsresource.Locality{
111+
{
112+
{{ID: clients.Locality{Zone: "L0"}}, {ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L2"}}},
113+
},
114+
{
115+
{{ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L2"}}},
116+
{{ID: clients.Locality{Zone: "L0"}}},
117+
},
118+
},
119+
want: []string{"priority-0-0", "priority-0-1"},
120+
},
121+
{
122+
name: "delete and bring back",
123+
inputs: [][][]xdsresource.Locality{
124+
{
125+
{{ID: clients.Locality{Zone: "L0"}}, {ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L2"}}},
126+
},
127+
{
128+
{{ID: clients.Locality{Zone: "L2"}}},
129+
{{ID: clients.Locality{Zone: "L1"}}},
130+
},
131+
{
132+
{{ID: clients.Locality{Zone: "L2"}}, {ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L0"}}},
133+
},
134+
},
135+
want: []string{"priority-0-0"},
136+
},
108137
{
109138
name: "complex merge split sequence",
110139
inputs: [][][]xdsresource.Locality{
@@ -140,7 +169,7 @@ func Test_nameGenerator_generate(t *testing.T) {
140169
{{ID: clients.Locality{Zone: "L3"}}},
141170
},
142171
},
143-
want: []string{"priority-0-0", "priority-0-1", "priority-0-2"},
172+
want: []string{"priority-0-0", "priority-0-3", "priority-0-2"},
144173
},
145174
{
146175
name: "complex full merges splits sequence",
@@ -198,7 +227,59 @@ func Test_nameGenerator_generate(t *testing.T) {
198227
{{ID: clients.Locality{Zone: "L1"}}},
199228
},
200229
},
201-
want: []string{"priority-0-1", "priority-0-0"},
230+
want: []string{"priority-0-0", "priority-0-2"},
231+
},
232+
{
233+
name: "merge-split reverse times 2",
234+
inputs: [][][]xdsresource.Locality{
235+
{
236+
{{ID: clients.Locality{Zone: "L1"}}},
237+
{{ID: clients.Locality{Zone: "L2"}}},
238+
},
239+
{
240+
{{ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L2"}}},
241+
},
242+
{
243+
{{ID: clients.Locality{Zone: "L2"}}},
244+
{{ID: clients.Locality{Zone: "L1"}}},
245+
},
246+
{
247+
{{ID: clients.Locality{Zone: "L1"}}},
248+
{{ID: clients.Locality{Zone: "L2"}}},
249+
},
250+
{
251+
{{ID: clients.Locality{Zone: "L1"}}, {ID: clients.Locality{Zone: "L2"}}},
252+
},
253+
{
254+
{{ID: clients.Locality{Zone: "L2"}}},
255+
{{ID: clients.Locality{Zone: "L1"}}},
256+
},
257+
},
258+
want: []string{"priority-0-2", "priority-0-0"},
259+
},
260+
{
261+
name: "complex merge-split reverse",
262+
inputs: [][][]xdsresource.Locality{
263+
{
264+
{{ID: clients.Locality{Zone: "L1"}}},
265+
{{ID: clients.Locality{Zone: "L2"}}},
266+
{{ID: clients.Locality{Zone: "L3"}}, {ID: clients.Locality{Zone: "L4"}}},
267+
},
268+
{
269+
{{ID: clients.Locality{Zone: "L1"}}},
270+
{{ID: clients.Locality{Zone: "L4"}}, {ID: clients.Locality{Zone: "L2"}}},
271+
{{ID: clients.Locality{Zone: "L3"}}},
272+
},
273+
{
274+
{{ID: clients.Locality{Zone: "L2"}}, {ID: clients.Locality{Zone: "L1"}}},
275+
{{ID: clients.Locality{Zone: "L4"}}},
276+
},
277+
{
278+
{{ID: clients.Locality{Zone: "L4"}}},
279+
{{ID: clients.Locality{Zone: "L1"}}},
280+
},
281+
},
282+
want: []string{"priority-0-4", "priority-0-2"},
202283
},
203284
{
204285
name: "2 by 2 shuffle sequence",

0 commit comments

Comments
 (0)