Skip to content

Commit

Permalink
kv: delete some paranoia
Browse files Browse the repository at this point in the history
Before this patch, Transaction.Clone() was not making a copy of an
empty, but with capacity, splice. This seems dangerous to me.
There was also some paranoia about it in the DistSender that can be
removed if cloning does the right thing.
  • Loading branch information
andreimatei committed May 11, 2017
1 parent 3345f38 commit b3085fd
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 7 deletions.
7 changes: 1 addition & 6 deletions pkg/kv/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,7 @@ func (ds *DistSender) initAndVerifyBatch(
//
// We already have a clone of our txn (see above), so we can
// modify it freely.
//
// Zero the existing data. That makes sure that if we had
// something of size zero but with capacity, we don't re-use the
// existing space (which others may also use). This is just to
// satisfy paranoia/OCD and not expected to matter in practice.
ba.Txn.ResetObservedTimestamps()

// OrigTimestamp is the HLC timestamp at which the Txn started, so
// this effectively means no more uncertainty on this node.
ba.Txn.UpdateObservedTimestamp(nDesc.NodeID, ba.Txn.OrigTimestamp)
Expand Down
2 changes: 1 addition & 1 deletion pkg/roachpb/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func (t Transaction) LastActive() hlc.Timestamp {
// keys with the intent spans.
func (t Transaction) Clone() Transaction {
mt := t.ObservedTimestamps
if len(mt) != 0 {
if mt != nil {
t.ObservedTimestamps = make([]ObservedTimestamp, len(mt))
copy(t.ObservedTimestamps, mt)
}
Expand Down

0 comments on commit b3085fd

Please sign in to comment.