From e3a93f21990b37e24080ae471be7fe63ae5cbf39 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Tue, 25 Sep 2018 13:54:17 +0200 Subject: [PATCH 1/2] sql: unskip TestSplitAt The failure was fixed in #29324. Closes #29169. Release note: None --- pkg/sql/split_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/sql/split_test.go b/pkg/sql/split_test.go index ca29c575cc50..f43543fc6e6d 100644 --- a/pkg/sql/split_test.go +++ b/pkg/sql/split_test.go @@ -31,8 +31,6 @@ import ( func TestSplitAt(t *testing.T) { defer leaktest.AfterTest(t)() - t.Skip("TODO(benesch): #29169: will be fixed by #29324") - params, _ := tests.CreateTestServerParams() s, db, _ := serverutils.StartServer(t, params) defer s.Stopper().Stop(context.TODO()) From 85d5ab68e6f3e3457db659c829e3da6a87c35f2b Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Tue, 25 Sep 2018 14:53:20 +0200 Subject: [PATCH 2/2] storage: fix data race in contentionQueue When a pusher would write a new intent, it would previously sent its own *TxnMeta to the next pusher. However, it would also possibly mutate it higher up the stack (for example combining multiple responses in DistSender). Make a copy instead. Release note: None --- pkg/storage/intent_resolver.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/storage/intent_resolver.go b/pkg/storage/intent_resolver.go index 9b98f4d5c9fc..06701466ff02 100644 --- a/pkg/storage/intent_resolver.go +++ b/pkg/storage/intent_resolver.go @@ -347,6 +347,13 @@ func (cq *contentionQueue) add( // contended key (i.e. newWIErr != nil). contended.setLastTxnMeta(nil) } + if newIntentTxn != nil { + // Shallow copy the TxnMeta. After this request returns (i.e. now), we might + // mutate it (DistSender and such), but the receiver of the channel will read + // it. + newIntentTxnCopy := *newIntentTxn + newIntentTxn = &newIntentTxnCopy + } curPusher.waitCh <- newIntentTxn close(curPusher.waitCh) cq.mu.Unlock()