Skip to content

Commit c56df25

Browse files
committed
more tests
Signed-off-by: Justin Jung <[email protected]>
1 parent d3018ca commit c56df25

File tree

3 files changed

+32
-10
lines changed

3 files changed

+32
-10
lines changed

pkg/ring/replication_set.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ track:
8181
tracker.done(res.instance, res.res, res.err)
8282
if res.err != nil {
8383
if tracker.failed() {
84-
if !partialDataEnabled || tracker.failedInAllZones() {
84+
if !partialDataEnabled || tracker.failedCompletely() {
8585
return nil, res.err
8686
}
8787
trackerFailed = true

pkg/ring/replication_set_tracker.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ type replicationSetResultTracker interface {
1515
// Returns list of instance addresses that failed
1616
failedInstances() []string
1717

18-
// Returns true if executions failed in all zones. Only relevant for zoneAwareResultTracker.
19-
failedInAllZones() bool
18+
// Returns true if executions failed in all instances or all zones.
19+
failedCompletely() bool
2020

2121
// Returns recorded results.
2222
getResults() []interface{}
@@ -29,6 +29,7 @@ type defaultResultTracker struct {
2929
maxErrors int
3030
failedInst []string
3131
results []interface{}
32+
numInstances int
3233
}
3334

3435
func newDefaultResultTracker(instances []InstanceDesc, maxErrors int) *defaultResultTracker {
@@ -39,6 +40,7 @@ func newDefaultResultTracker(instances []InstanceDesc, maxErrors int) *defaultRe
3940
maxErrors: maxErrors,
4041
failedInst: make([]string, 0, len(instances)),
4142
results: make([]interface{}, 0, len(instances)),
43+
numInstances: len(instances),
4244
}
4345
}
4446

@@ -60,8 +62,8 @@ func (t *defaultResultTracker) failed() bool {
6062
return t.numErrors > t.maxErrors
6163
}
6264

63-
func (t *defaultResultTracker) failedInAllZones() bool {
64-
return false
65+
func (t *defaultResultTracker) failedCompletely() bool {
66+
return t.numInstances == t.numErrors
6567
}
6668

6769
func (t *defaultResultTracker) failedInstances() []string {
@@ -141,7 +143,7 @@ func (t *zoneAwareResultTracker) failed() bool {
141143
return failedZones > t.maxUnavailableZones
142144
}
143145

144-
func (t *zoneAwareResultTracker) failedInAllZones() bool {
146+
func (t *zoneAwareResultTracker) failedCompletely() bool {
145147
failedZones := len(t.failuresByZone)
146148
return failedZones == t.zoneCount
147149
}

pkg/ring/replication_set_tracker_test.go

+24-4
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,26 @@ func TestDefaultResultTracker(t *testing.T) {
154154
assert.Equal(t, []interface{}{[]int{1, 1, 1}, []int{2, 2, 2}, []int{3, 3, 3}}, tracker.getResults())
155155
},
156156
},
157+
"failedCompletely should return true only if all instances have failed, regardless of max errors": {
158+
instances: []InstanceDesc{instance1, instance2, instance3},
159+
maxErrors: 1,
160+
run: func(t *testing.T, tracker *defaultResultTracker) {
161+
tracker.done(&instance1, nil, errors.New("test"))
162+
assert.False(t, tracker.succeeded())
163+
assert.False(t, tracker.failed())
164+
assert.False(t, tracker.failedCompletely())
165+
166+
tracker.done(&instance2, nil, errors.New("test"))
167+
assert.False(t, tracker.succeeded())
168+
assert.True(t, tracker.failed())
169+
assert.False(t, tracker.failedCompletely())
170+
171+
tracker.done(&instance3, nil, errors.New("test"))
172+
assert.False(t, tracker.succeeded())
173+
assert.True(t, tracker.failed())
174+
assert.True(t, tracker.failedCompletely())
175+
},
176+
},
157177
"failedInstances should work": {
158178
instances: []InstanceDesc{instance1, instance2},
159179
maxErrors: 2,
@@ -410,27 +430,27 @@ func TestZoneAwareResultTracker(t *testing.T) {
410430
assert.False(t, tracker.failed())
411431
},
412432
},
413-
"failInAllZones should return true only if all zones have failed, regardless of max unavailable zones": {
433+
"failedCompletely should return true only if all zones have failed, regardless of max unavailable zones": {
414434
instances: []InstanceDesc{instance1, instance2, instance3, instance4, instance5, instance6},
415435
maxUnavailableZones: 1,
416436
run: func(t *testing.T, tracker *zoneAwareResultTracker) {
417437
// Zone-a
418438
tracker.done(&instance1, nil, errors.New("test"))
419439
assert.False(t, tracker.succeeded())
420440
assert.False(t, tracker.failed())
421-
assert.False(t, tracker.failedInAllZones())
441+
assert.False(t, tracker.failedCompletely())
422442

423443
// Zone-b
424444
tracker.done(&instance3, nil, errors.New("test"))
425445
assert.False(t, tracker.succeeded())
426446
assert.True(t, tracker.failed())
427-
assert.False(t, tracker.failedInAllZones())
447+
assert.False(t, tracker.failedCompletely())
428448

429449
// Zone-c
430450
tracker.done(&instance5, nil, errors.New("test"))
431451
assert.False(t, tracker.succeeded())
432452
assert.True(t, tracker.failed())
433-
assert.True(t, tracker.failedInAllZones())
453+
assert.True(t, tracker.failedCompletely())
434454
},
435455
},
436456
"failedInstances should work": {

0 commit comments

Comments
 (0)