Skip to content

Commit

Permalink
kv: fix data race during retry of EndTxn after refresh
Browse files Browse the repository at this point in the history
Fixes cockroachdb#103687.
Fixes cockroachdb#103247.
Fixes cockroachdb#104791.

This commit avoids a data race between `splitEndTxnAndRetrySend` and
`raceTransport` by avoiding a mutation of a shared `RequestUnion_EndTxn`
object within an unshared `RequestUnion` object. The `raceTransport`
makes an effort to copy the `BatchRequest`'s `RequestUnion` slice, but
it does not copy the inner interface, so we can't play tricks to avoid a
reallocation of the `RequestUnion_EndTxn`.

The commit also addresses a similar problem in
`retryTxnCommitAfterFailedParallelCommit`.

We may be able to fix this in the `raceTransport`, but doing so would
require some reflection magic and this is currently failing CI, so we
make the easier change.

Release note: None
  • Loading branch information
nvanbenschoten committed Jun 24, 2023
1 parent 98050cf commit e29a2bc
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ func (tc *txnCommitter) retryTxnCommitAfterFailedParallelCommit(
et := baSuffix.Requests[0].GetEndTxn().ShallowCopy().(*kvpb.EndTxnRequest)
et.LockSpans, _ = mergeIntoSpans(et.LockSpans, et.InFlightWrites)
et.InFlightWrites = nil
baSuffix.Requests[0].Value.(*kvpb.RequestUnion_EndTxn).EndTxn = et
baSuffix.Requests[0].MustSetInner(et)
}
brSuffix, pErr := tc.wrapped.SendLocked(ctx, baSuffix)
if pErr != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func (sr *txnSpanRefresher) splitEndTxnAndRetrySend(
et = et.ShallowCopy().(*kvpb.EndTxnRequest)
et.LockSpans, _ = mergeIntoSpans(et.LockSpans, et.InFlightWrites)
et.InFlightWrites = nil
baSuffix.Requests[0].Value.(*kvpb.RequestUnion_EndTxn).EndTxn = et
baSuffix.Requests[0].MustSetInner(et)
}
brSuffix, pErr := sr.SendLocked(ctx, baSuffix)
if pErr != nil {
Expand Down

0 comments on commit e29a2bc

Please sign in to comment.