Make the Rebind concurrency test deterministic
It asserted that a reader had to observe both of the swapped-in upstream addresses, which is a demand about *which* list a reader happens to see. A reader that finished its sampling before the rebind loop got there satisfied every real property and still failed, so this failed roughly 2 runs in 10 and would have turned CI red on nothing. What the test should pin is the copy-on-write invariant itself: a reader must only ever see a whole, published list. Each reader now compares every observed snapshot against the three lists this test publishes, and fails immediately on a half-applied one. That is checked on every iteration rather than inferred afterwards, and it makes no demand about which list was seen, so it cannot flake. 15 consecutive -race runs, no failure.
This commit is contained in:
1 parent
a5e54c09df
commit
16c8ad27b9
1 file changed
+29
-7
@@ -496,6 +496,9 @@ func TestFailoverPolicyWalksTheListInOrder(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// joinAddrs builds the map key for a whole upstream list.
|
||||
func joinAddrs(addrs ...string) string { return strings.Join(addrs, ",") }
|
||||
|
||||
// TestRebindUnderConcurrentQueries hammers Rebind while queries, health
|
||||
// snapshots and latency reads run in parallel, so -race covers the copy-on-write
|
||||
// hand-over. Afterwards the live state of an unchanged server must have
|
||||
@@ -518,7 +521,17 @@ func TestRebindUnderConcurrentQueries(t *testing.T) {
|
||||
queriesBefore := before.Stats().Queries
|
||||
|
||||
stop := make(chan struct{})
|
||||
var seen sync.Map // address -> true, every list the readers ever observed
|
||||
var observed atomic.Int64
|
||||
// legal is every upstream list this test publishes. A reader must only
|
||||
// ever see one of them whole: never both b and c, never an empty list.
|
||||
// That is the copy-on-write invariant worth pinning, and unlike "a reader
|
||||
// must happen to observe b and c" it cannot flake, because it makes no
|
||||
// demand about which list was seen — only that it was a real one.
|
||||
legal := map[string]bool{
|
||||
joinAddrs(a.addr): true,
|
||||
joinAddrs(a.addr, b.addr): true,
|
||||
joinAddrs(a.addr, c.addr): true,
|
||||
}
|
||||
var wg sync.WaitGroup
|
||||
for r := 0; r < 4; r++ {
|
||||
wg.Add(1)
|
||||
@@ -530,8 +543,19 @@ func TestRebindUnderConcurrentQueries(t *testing.T) {
|
||||
return
|
||||
default:
|
||||
}
|
||||
for _, u := range p.All() {
|
||||
seen.Store(u.Addr(), true)
|
||||
all := p.All()
|
||||
observed.Add(1)
|
||||
if len(all) == 0 {
|
||||
t.Error("All() returned an empty pool during a Rebind")
|
||||
return
|
||||
}
|
||||
addrs := make([]string, 0, len(all))
|
||||
for _, u := range all {
|
||||
addrs = append(addrs, u.Addr())
|
||||
}
|
||||
if !legal[strings.Join(addrs, ",")] {
|
||||
t.Errorf("reader observed a half-applied upstream list: %v", addrs)
|
||||
return
|
||||
}
|
||||
p.Healthy()
|
||||
p.Stats()
|
||||
@@ -580,10 +604,8 @@ func TestRebindUnderConcurrentQueries(t *testing.T) {
|
||||
if got := len(p.All()); got != 2 {
|
||||
t.Errorf("pool holds %d upstreams, want 2", got)
|
||||
}
|
||||
for _, addr := range []string{b.addr, c.addr} {
|
||||
if _, ok := seen.Load(addr); !ok {
|
||||
t.Errorf("no reader ever observed %s while Rebind swapped the list", addr)
|
||||
}
|
||||
if observed.Load() == 0 {
|
||||
t.Error("the readers never sampled the pool, so nothing was actually tested")
|
||||
}
|
||||
if !strings.Contains(p.String(), c.addr) {
|
||||
t.Errorf("pool %s does not mention the new upstream %s", p, c.addr)
|
||||
|
||||
Reference in new issue
Block a user