Skip to content

Commit 21a2810

Browse files
committed
asim: correctly update queue.next in pushReplicateChange
Previously, we did not assign the next field of the queues correctly because we passed the base queue by value into pushReplicateChange, which caused updates to the next field to affect only the copy, not the original queue. This commit fixes the issue by returning the correct next value and updating the queue directly. Epic: none Release note: none
1 parent af3c9cc commit 21a2810

File tree

4 files changed

+59
-50
lines changed

4 files changed

+59
-50
lines changed

pkg/kv/kvserver/asim/queue/lease_queue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (lq *leaseQueue) Tick(ctx context.Context, tick time.Time, s state.State) {
158158
continue
159159
}
160160

161-
pushReplicateChange(
161+
lq.next = pushReplicateChange(
162162
ctx, change, rng, tick, lq.settings.ReplicaChangeDelayFn(), lq.baseQueue)
163163
}
164164

pkg/kv/kvserver/asim/queue/replicate_queue.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func (rq *replicateQueue) Tick(ctx context.Context, tick time.Time, s state.Stat
152152
continue
153153
}
154154

155-
pushReplicateChange(
155+
rq.next = pushReplicateChange(
156156
ctx, change, rng, tick, rq.settings.ReplicaChangeDelayFn(), rq.baseQueue)
157157
}
158158

@@ -166,12 +166,13 @@ func pushReplicateChange(
166166
tick time.Time,
167167
delayFn func(int64, bool) time.Duration,
168168
queue baseQueue,
169-
) {
169+
) time.Time {
170170
var stateChange state.Change
171+
next := tick
171172
switch op := change.Op.(type) {
172173
case plan.AllocationNoop:
173174
// Nothing to do.
174-
return
175+
return next
175176
case plan.AllocationFinalizeAtomicReplicationOp:
176177
panic("unimplemented finalize atomic replication op")
177178
case plan.AllocationTransferLeaseOp:
@@ -194,9 +195,10 @@ func pushReplicateChange(
194195
}
195196

196197
if completeAt, ok := queue.stateChanger.Push(tick, stateChange); ok {
197-
queue.next = completeAt
198198
log.VEventf(ctx, 1, "pushing state change succeeded, complete at %s (cur %s)", completeAt, tick)
199+
next = completeAt
199200
} else {
200201
log.VEventf(ctx, 1, "pushing state change failed")
201202
}
203+
return next
202204
}

pkg/kv/kvserver/asim/tests/testdata/non_rand/example_add_node.txt

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,31 +28,38 @@ set_span_config
2828

2929
eval duration=20m samples=1 seed=42
3030
----
31-
OK
31+
failed assertion sample 1
32+
balance stat=replicas threshold=(<1.05) ticks=6
33+
max/mean=1.08 tick=0
34+
max/mean=1.08 tick=1
35+
max/mean=1.08 tick=2
36+
max/mean=1.08 tick=3
37+
max/mean=1.08 tick=4
38+
max/mean=1.08 tick=5
3239

3340
# Plot the replica count from the evaluation. Since there are 300 replicas on
3441
# s1 and the default RF=3, we expect the other stores to be up-replicated to
3542
# 300 replicas as well.
3643
plot stat=replicas sample=1
3744
----
38-
301 ┼──────────────────────────────────────────────────────────────────────────────
39-
281 ┤ ╭──╯
40-
261 ┤ ╭─╯╯
41-
241 ┤ ──╯
42-
221 ┤ ╭╭─╯
43-
201 ┤ ╭╭──╯
44-
181 ┤ ╭╭─╯
45-
161 ┤ ╭╭──
46-
140 ┤ ──╯
47-
120 ┤ ╭╭─
48-
100 ┤ ╭╭──╯
49-
80 ┤ ╭──╯
50-
60 ┤ ──╯
51-
40 ┤ ╭─╯
52-
20 ┤ ╭──
53-
0 ┼─╯
45+
301 ┼──────────────────────────────────────────────────────────────────────────────
46+
281 ┤
47+
261 ┤ ╭─╭───────
48+
241 ┤ ╭──────╯
49+
221 ┤ ╭────╯
50+
201 ┤ ╭╭────╯
51+
181 ┤ ╭──────╯
52+
161 ┤ ╭────╯
53+
140 ┤ ╭───╯
54+
120 ┤ ╭──╯
55+
100 ┤ ╭────╯
56+
80 ┤ ╭──
57+
60 ┤ ╭╭────╯
58+
40 ┤ ╭────────
59+
20 ┤ ╭╭───────╯
60+
0 ┼─────
5461
replicas
5562
initial store values: [s1=301, s2=0, s3=0] (stddev=141.89, mean=100.33, sum=301)
56-
last store values: [s1=300, s2=300, s3=301] (stddev=0.47, mean=300.33, sum=901)
63+
last store values: [s1=301, s2=272, s3=265] (stddev=15.58, mean=279.33, sum=838)
5764

5865
# vim:ft=sh

pkg/kv/kvserver/asim/tests/testdata/non_rand/example_fulldisk.txt

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,46 +23,46 @@ OK
2323
# store 5. This is shown below as it sheds replicas.
2424
plot stat=replicas
2525
----
26-
347╭╮ ╭╮╭╭╮ ╭╮╭──╭╮─╭╮─╭╮─╭──╮╮╭
27-
334╭╮ ╭╮ ╭╮ ╭─╮╮╭╮──╭╮╮╭╮─╭───╮─╭───────╮╭──╮╭─╯╰╮╭──╯╰╮╭─╯╰─╯╰─╯╰─╯─╯│╰╭
28-
321 ┤ ╭───╭──╮╭╭───────╯╭│╭────╯╰─╯╰─╯╮╭─╰─╯╰╯╮╭╰─╯╰╯─╯╰╯╯╰─╰╯╰╮╭╯╰╯ ╰╯╰─╯ ╰─
29-
307 ┼╮────╯╯╯╰─╯╰╯╯╰╯╯ ╰╯╰╯╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰
30-
294 ┤╰╮
31-
281
32-
268╰╮╭╮ ╭
33-
255 ┤ ╰╯╰─╯│╭─╮ ╭
34-
241╰╯ ╰─╯│╭─╮╭─╮ ╭
35-
228╰╯ ╰╯ │╭╯╰╮╭╮ ╭╮ ╭
36-
215╰╯╯╰─╯╰─╯╰╮╭─╮ ╭
37-
202 ╰╯ │╭─╮ │╰╮╭╮ ╭─╮ ╭╮ ╭
38-
189╰╯ ╰─╯ ╰╯╰╮│ │ │╰╮╭─╮ │╰╮ ╭╮ ╭
39-
175 ┤ ╰╯ ╰─╯ ╰╯ ╰─╯ ╰─╯╰╮ │╰╮╭─╮ ╭─╮ ╭
40-
162 ╰─╯ ╰╯ │ │ │ │ ╰
41-
149 ╰─╯ ╰─╯
26+
344 ╭╮ ╭╮╭╭╮╮╭─╮╮╭╮─╭─╮╭──
27+
331 ╭╮ ╭╮╭─╮╭╮──╭──╮╭──────────────────────╯╰─╯╰─╯╰────╯╯─
28+
318 ╭───╭─╮╮╭────────────────╯╯╰╯╰──╯╰╰╰╯╯ ╰
29+
305 ┼╮──────╯╯╰─╯╯
30+
293 ┤╰╮
31+
280╰─╮
32+
267
33+
255 ┤ ╰────
34+
242 ╰─────
35+
229 ╰─────
36+
217 ┤ ╰───────
37+
204╰──────────
38+
191 ╰────
39+
178 ┤ ╰───────────╮ ╭╮
40+
166 ┤ ╰─╯╰──────╮╭──╮ ╭╮
41+
153╰╯ ╰─╯╰────
4242
replicas
4343
initial store values: [s1=300, s2=300, s3=300, s4=300, s5=300] (stddev=0.00, mean=300.00, sum=1500)
44-
last store values: [s1=347, s2=342, s3=338, s4=336, s5=152] (stddev=75.59, mean=303.00, sum=1515)
44+
last store values: [s1=342, s2=336, s3=339, s4=339, s5=153] (stddev=74.42, mean=301.80, sum=1509)
4545

4646
# Plot the % of disk storage capacity used. We should see s5 hovering right
4747
# around 92.5-95% (the storage capacity threshold value).
4848
plot stat=disk_fraction_used
4949
----
50-
1.05 ┼╮ ╭╮ ╭╮ ╭
51-
0.99 ┤╰╮ ╭╮ ╭╮ ╭╮ ╭╮ ╭╮ ╭╮ ╭╮ ╭─╮ ╭╮ ╭╮ │╰╮ ││ ││ ╭╮ ╭─╮ ╭─╮
52-
0.94 ┤ ╰╮╭╮ ╭╮╭─╮╭─╮╭─╮╭─╮╭╯╰╮│╰╮│╰─╯╰╮│╰╮╭─╮ │╰╮││ │ │ │╰╮╭╯╰╮│ ╰─╯╰╮╭╯╰╮│╰╮ │ │ │ │
53-
0.88 ┤ ╰╯╰─╯╰╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰╯ │╭╯ ╰╯╰─╯ ╰─╯ ╰╯ ╰╯ ╰╯ ╰╯ ╰─╯ ╰─╯ ╰
54-
0.82 ┤ ╰╯
50+
1.05 ┼
51+
0.99 ┤─╮
52+
0.94 ┤ ╰─────────────────╮╭─╮╭──╮╭────────────────────╮╭────╮ ╭───────╮╭──╮╭──────
53+
0.88 ┤ ╰╯ ╰╯ ╰╯ ╰╯ ╰─╯ ╰╯ ╰╯
54+
0.82 ┤
5555
0.77 ┤
5656
0.71 ┤
5757
0.65 ┤
5858
0.60 ┤
5959
0.54 ┤
6060
0.49 ┤
6161
0.43 ┤
62-
0.37 ┤ ╭╮╭─╭───────────────
63-
0.32 ┤ ╭──╭───────────────────╯╰──╯╰
64-
0.26 ┤ ╭╮╭╮╭─╮╭─────────────────╯╯─
65-
0.20 ┼────────────────╯╰╯╯╰╯╯
62+
0.37 ┤ ╭╭╭────────────────
63+
0.32 ┤ ╭─────────────────────╯╰╯
64+
0.26 ┤ ╭───────────────────────╯
65+
0.20 ┼────────────────╯╯╯
6666
disk_fraction_used
6767
initial store values: [s1=0.20, s2=0.20, s3=0.20, s4=0.20, s5=1.05] (stddev=0.34, mean=0.37, sum=2)
68-
last store values: [s1=0.41, s2=0.41, s3=0.40, s4=0.40, s5=0.93] (stddev=0.21, mean=0.51, sum=3)
68+
last store values: [s1=0.41, s2=0.40, s3=0.41, s4=0.41, s5=0.94] (stddev=0.22, mean=0.51, sum=3)

0 commit comments

Comments
 (0)