Skip to content

Commit

Permalink
roachpb: make the ignored seq num list copy on write
Browse files Browse the repository at this point in the history
Unfortunately the RaceTransport doesn't allow any data referenced by a
Transaction proto to be mutated. This patch makes updates to the ignored
seq nums conform.
The RaceTransport wants to prevent the server from doing such mutations.
In this case it's the client that does the mutations, which should be
allowed, but the implementation of this testing tool casts a wide net.

Release note: None
  • Loading branch information
andreimatei committed Mar 5, 2020
1 parent 9a2f046 commit b8af030
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 55 deletions.
24 changes: 13 additions & 11 deletions pkg/roachpb/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -1162,8 +1162,10 @@ func (t *Transaction) GetObservedTimestamp(nodeID NodeID) (hlc.Timestamp, bool)
return s.get(nodeID)
}

// AddIgnoredSeqNumRange adds the given range to the transaction's list of
// ignored seqnum ranges.
// AddIgnoredSeqNumRange adds the given range to the given list of
// ignored seqnum ranges. Since none of the references held by a Transaction
// allow interior mutations, the existing list is copied instead of being
// mutated in place.
//
// The following invariants are assumed to hold and are preserved:
// - the list contains no overlapping ranges
Expand Down Expand Up @@ -1196,16 +1198,16 @@ func (t *Transaction) GetObservedTimestamp(nodeID NodeID) (hlc.Timestamp, bool)
// previously seen value.
func (t *Transaction) AddIgnoredSeqNumRange(newRange enginepb.IgnoredSeqNumRange) {
// Truncate the list at the last element not included in the new range.

list := t.IgnoredSeqNums
i := 0
for ; i < len(list); i++ {
if list[i].End < newRange.Start {
continue
}
break
}
list = list[:i]
t.IgnoredSeqNums = append(list, newRange)
i := sort.Search(len(list), func(i int) bool {
return list[i].End >= newRange.Start
})

cpy := make([]enginepb.IgnoredSeqNumRange, i+1)
copy(cpy[:i], list[:i])
cpy[i] = newRange
t.IgnoredSeqNums = cpy
}

// AsRecord returns a TransactionRecord object containing only the subset of
Expand Down
61 changes: 31 additions & 30 deletions pkg/roachpb/data.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions pkg/roachpb/data.proto
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,11 @@ message Transaction {
repeated SequencedWrite in_flight_writes = 17 [(gogoproto.nullable) = false];
// A list of ignored seqnum ranges.
//
// The user code must guarantee this list to be non-overlapping,
// non-contiguous (i.e. it must coalesce ranges to avoid situations
// where a range's end seqnum is equal to the next range's start
// seqnum), and sorted in seqnum order.
// The slice is maintained as non-overlapping, non-contiguous (i.e. it must
// coalesce ranges to avoid situations where a range's end seqnum is equal to
// the next range's start seqnum), and sorted in seqnum order. It should be
// treated as immutable and all updates should be performed on a copy of the
// slice.
repeated storage.enginepb.IgnoredSeqNumRange ignored_seqnums = 18
[(gogoproto.nullable) = false, (gogoproto.customname) = "IgnoredSeqNums"];

Expand Down
30 changes: 20 additions & 10 deletions pkg/roachpb/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1948,21 +1948,31 @@ func TestAddIgnoredSeqNumRange(t *testing.T) {
newRange r
exp []r
}{
{[]r{},
{
[]r{},
mr(1, 2),
[]r{mr(1, 2)}},
{[]r{mr(1, 2)},
[]r{mr(1, 2)},
},
{
[]r{mr(1, 2)},
mr(1, 4),
[]r{mr(1, 4)}},
{[]r{mr(1, 2), mr(3, 6)},
[]r{mr(1, 4)},
},
{
[]r{mr(1, 2), mr(3, 6)},
mr(8, 10),
[]r{mr(1, 2), mr(3, 6), mr(8, 10)}},
{[]r{mr(1, 2), mr(5, 6)},
[]r{mr(1, 2), mr(3, 6), mr(8, 10)},
},
{
[]r{mr(1, 2), mr(5, 6)},
mr(3, 8),
[]r{mr(1, 2), mr(3, 8)}},
{[]r{mr(1, 2), mr(5, 6)},
[]r{mr(1, 2), mr(3, 8)},
},
{
[]r{mr(1, 2), mr(5, 6)},
mr(1, 8),
[]r{mr(1, 8)}},
[]r{mr(1, 8)},
},
}

for _, tc := range testData {
Expand Down

0 comments on commit b8af030

Please sign in to comment.