Skip to content

Commit 2bda2d8

Browse files
craig[bot]pav-kv
andcommitted
Merge #143191
143191: kvserver: reset some fields of replicaAppBatch r=tbg a=pav-kv This commit fixes a bug in `replicaAppBatch` initialization. Some fields were not reset, and would be carried across batches. For example, the `changeTruncatesSideloadedFiles` field, once set to true, would be true forever, and cause Pebble sync on every command application. Epic: none Release note: none Co-authored-by: Pavel Kalinnikov <[email protected]>
2 parents 4f514e3 + a5de0c2 commit 2bda2d8

File tree

1 file changed

+6
-0
lines changed

1 file changed

+6
-0
lines changed

pkg/kv/kvserver/replica_application_state_machine.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ func (sm *replicaStateMachine) NewEphemeralBatch() apply.EphemeralBatch {
138138
func (sm *replicaStateMachine) NewBatch() apply.Batch {
139139
r := sm.r
140140
b := &sm.batch
141+
// TODO(pav-kv): replicaAppBatch initialization below is bug-prone, we need to
142+
// not forget resetting the fields that are local to one batch. Find a way to
143+
// make it safer.
141144
b.r = r
142145
b.applyStats = &sm.applyStats
143146
b.batch = r.store.TODOEngine().NewBatch()
@@ -148,6 +151,9 @@ func (sm *replicaStateMachine) NewBatch() apply.Batch {
148151
*b.state.Stats = *r.shMu.state.Stats
149152
b.closedTimestampSetter = r.mu.closedTimestampSetter
150153
r.mu.RUnlock()
154+
b.changeRemovesReplica = false
155+
b.changeTruncatesSideloadedFiles = false
156+
// TODO(pav-kv): what about b.ab and b.followerStoreWriteBytes?
151157
b.start = timeutil.Now()
152158
return b
153159
}

0 commit comments

Comments
 (0)