cockroachdb / cockroachdb/cockroach

kvserver: range stats mismatch because of concurrent transaction record mutations

Open
#152,164 0 comments 0 reactions 0 assignees View on GitHub
A-kv-transactions branch-master C-bug P-2 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

The following theory hasn't yet been 100% confirmed.

Both `EndTxn(abort)` and `GCRequest` may attempt to update/delete a transaction key. However, given the current latching rules, I do not believe these requests are isolated from each other.

An EndTxn(abort) request (that doesn't have an internal commit trigger), takes out the following latches:

```golang
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{
Key: keys.TransactionKey(req.Header().Key, header.Txn.ID),
})
...
latchSpans.AddNonMVCC(abortSpanAccess, roachpb.Span{
Key: keys.AbortSpanKey(rs.GetRangeID(), header.Txn.ID),
})
...
latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{
Key: keys.RangeDescriptorKey(rs.GetStartKey()),
})
...
for _, span := range et.LockSpans {
latchSpans.AddMVCC(spanset.SpanReadWrite, span, minTxnTS)
}
```

GCRequest with only Keys set will take out the following latches:

```golang
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.RangeGCThresholdKey(rs.GetRangeID())})
// Needed for Range bounds checks in calls to EvalContext.ContainsKey.
latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{Key: keys.RangeDescriptorKey(rs.GetStartKey())})
// Needed for updating optional GC hint.
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.RangeGCHintKey(rs.GetRangeID())})
```

Such GC requests are created by the intent resolver:

https://github.com/cockroachdb/cockroach/blob/7e7425f698b4f612a2e72cc8e813b44c52af89cb/pkg/kv/kvserver/intentresolver/intent_resolver.go#L794-L835

The only place these latch sets appear to intersect are the RangeDescriptorKey, which is ReadOnly in both cases and thus will happily coexist.

I believe we see this violation in some of the recent KVNemesis failures related to stats mismatches. For instance, in one case KVNemesis reports a SysBytes mismatch of -156 and a SysCount mismatch of -1. And, in the raft log we can see the following:

```
****** index 128 ******
RaftCommand: proposer_lease_sequence:2 max_lease_index:118 closed_timestamp: replicated_eval_result: delta: > logical_op_log:<>

Delete (Sized at 156): 0,0 /Local/Range/Table/100/"3467262c76eb7cc6"/Transaction/"7c27f2b5-6b21-4636-89ec-450dc86eb2cc" (0x016b12ec123334363732363263373665623763633600ff01000174786e2d7c27f2b56b21463689ec450dc86eb2cc00):
****** index 129 ******
RaftCommand: proposer_lease_sequence:2 max_lease_index:119 closed_timestamp: replicated_eval_result: delta: > logical_op_log:<>
Delete (Sized at 156): 0,0 /Local/Range/Table/100/"3467262c76eb7cc6"/Transaction/"7c27f2b5-6b21-4636-89ec-450dc86eb2cc" (0x016b12ec123334363732363263373665623763633600ff01000174786e2d7c27f2b56b21463689ec450dc86eb2cc00):
```

Two consecutive entries in the raft log both deleting the same transaction key. Had these requests been isolated, one assumes that the EndTxn that I believe produced the second entry (based on the abscense of a last_update_nanos) would not have found the transaction key and not have issued a delete.

The pebble logs additionally contain a warning that hints at the fact that we have indeed issued a sized delete against a key that doesn't exist:

```
I250819 14:33:36.878273 22343 3@pebble/event.go:1277 ⋮ [n2,s2,pebble] 8135 possible API misuse: missized DELSIZED (key=‹"\x01k\x12\xec\x123467262c76eb7cc6\x00\xff\x01\x00\x01txn-|'\xf2\xb5k!F6\x89\xecE\r\xc8n\xb2\xcc\x00"›, elidedSize=0,expected
Size=0)
```

Analysis of at least 3 other stats mismatches shows similar raft log entries. I believe this is the cause of at least some of the stats mismatches from KVNemesis and may have existed since https://github.com/cockroachdb/cockroach/pull/83213 merged. However, it isn't clear why we are seeing this more often lately.

One possible solution here is to at the very least take out latches on individual keys that are specified in GCRequest.Keys in some cases. We'll need to work to preserve the properties we were aiming for in 83213.

Jira issue: CRDB-53687

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.