Skip to content

Commit ae644bb

Browse files
committed
kvserver: fix locking in mma_replica_store
Previously, `checkIfSpanConfigNeedsAnUpdate` did not properly hold a lock, and `markSpanConfigNeedsUpdateLocked` incorrectly set `r.mu.mmaSpanConfigIsUpToDate` to true instead of false. This commit fixes both issues and renames `checkIfSpanConfigNeedsAnUpdate` to `maybePromiseSpanConfigUpdate` for clarity. In addition, it adds `r.mu.AssertHeld()` to `markSpanConfigNeedsUpdateLocked`.
1 parent b451fa8 commit ae644bb

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

pkg/kv/kvserver/mma_replica_store.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,10 @@ func (mr *mmaReplica) mmaRangeLoad() mmaprototype.RangeLoad {
4141
return mmaRangeLoad(r.LoadStats(), r.GetMVCCStats())
4242
}
4343

44-
// checkIfSpanConfigNeedsAnUpdate determines whether an up-to-date span config
45-
// should be sent to mma. The contract is: if this returns true, mmaReplica must
46-
// send a fully populated range message to mma.
44+
// maybePromiseSpanConfigUpdate determines whether an up-to-date span config
45+
// should be sent to mma and overrides mmaSpanConfigIsUpToDate as true. The
46+
// contract is: if this returns true, mmaReplica must send a fully populated
47+
// range message to mma.
4748
//
4849
// Ideally, this would be called within isLeaseholderWithDescAndConfig so that we
4950
// acquire the lock on the replica only once. However, we can't do that because,
@@ -56,8 +57,10 @@ func (mr *mmaReplica) mmaRangeLoad() mmaprototype.RangeLoad {
5657
// it may decide to drop the span config even though the most up-to-date span
5758
// config might end up being dropped. In addition, isLeaseholderWithDescAndConfig
5859
// would only need a read lock on the replica without this.
59-
func (mr *mmaReplica) checkIfSpanConfigNeedsAnUpdate() (needed bool) {
60+
func (mr *mmaReplica) maybePromiseSpanConfigUpdate() (needed bool) {
6061
r := (*Replica)(mr)
62+
r.mu.Lock()
63+
defer r.mu.Unlock()
6164
needed = !r.mu.mmaSpanConfigIsUpToDate
6265
r.mu.mmaSpanConfigIsUpToDate = true
6366
return needed
@@ -81,7 +84,8 @@ func (mr *mmaReplica) markSpanConfigNeedsUpdate() {
8184
// sending a full range message to mma.
8285
func (mr *mmaReplica) markSpanConfigNeedsUpdateLocked() {
8386
r := (*Replica)(mr)
84-
r.mu.mmaSpanConfigIsUpToDate = true
87+
r.mu.AssertHeld()
88+
r.mu.mmaSpanConfigIsUpToDate = false
8589
}
8690

8791
// isLeaseholderWithDescAndConfig checks if the replica is the leaseholder and
@@ -208,7 +212,7 @@ func (mr *mmaReplica) tryConstructMMARangeMsg(
208212
// At this point, we know r is the leaseholder replica.
209213
replicas := constructMMAUpdate(desc, raftStatus, r.StoreID() /*leaseholderReplicaStoreID*/)
210214
rLoad := mr.mmaRangeLoad()
211-
if mr.checkIfSpanConfigNeedsAnUpdate() {
215+
if mr.maybePromiseSpanConfigUpdate() {
212216
return true, false, mmaprototype.RangeMsg{
213217
RangeID: r.RangeID,
214218
Replicas: replicas,

0 commit comments

Comments
 (0)