Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-20.1: kv/concurrency: permit lock timestamp regression across durabilities #47139

Merged
merged 1 commit into from
Apr 7, 2020
Merged

release-20.1: kv/concurrency: permit lock timestamp regression across durabilities #47139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/kv/kvserver/concurrency/concurrency_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ import (
// handle-write-intent-error req=<req-name> txn=<txn-name> key=<key>
// handle-txn-push-error req=<req-name> txn=<txn-name> key=<key> TODO(nvanbenschoten): implement this
//
// on-lock-acquired req=<req-name> key=<key> [seq=<seq>]
// on-lock-acquired req=<req-name> key=<key> [seq=<seq>] [dur=r|u]
// on-lock-updated req=<req-name> txn=<txn-name> key=<key> status=[committed|aborted|pending] [ts=<int>[,<int>]]
// on-txn-updated txn=<txn-name> status=[committed|aborted|pending] [ts=<int>[,<int>]]
//
Expand Down Expand Up @@ -276,6 +276,11 @@ func TestConcurrencyManagerBasic(t *testing.T) {
}
seqNum := enginepb.TxnSeq(seq)

dur := lock.Unreplicated
if d.HasArg("dur") {
dur = scanLockDurability(t, d)
}

// Confirm that the request has a corresponding write request.
found := false
for _, ru := range guard.Req.Requests {
Expand All @@ -298,7 +303,7 @@ func TestConcurrencyManagerBasic(t *testing.T) {
mon.runSync("acquire lock", func(ctx context.Context) {
log.Eventf(ctx, "txn %s @ %s", txn.ID.Short(), key)
span := roachpb.Span{Key: roachpb.Key(key)}
up := roachpb.MakeLockUpdateWithDur(txnAcquire, span, lock.Unreplicated)
up := roachpb.MakeLockUpdateWithDur(txnAcquire, span, dur)
m.OnLockAcquired(ctx, &up)
})
return c.waitAndCollect(t, mon)
Expand Down
15 changes: 15 additions & 0 deletions pkg/kv/kvserver/concurrency/datadriven_util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"strings"
"testing"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
Expand Down Expand Up @@ -58,6 +59,20 @@ func scanTimestampWithName(t *testing.T, d *datadriven.TestData, name string) hl
return ts
}

func scanLockDurability(t *testing.T, d *datadriven.TestData) lock.Durability {
var durS string
d.ScanArgs(t, "dur", &durS)
switch durS {
case "r":
return lock.Replicated
case "u":
return lock.Unreplicated
default:
d.Fatalf(t, "unknown lock durability: %s", durS)
return 0
}
}

func scanSingleRequest(
t *testing.T, d *datadriven.TestData, line string, txns map[string]*roachpb.Transaction,
) roachpb.Request {
Expand Down
31 changes: 25 additions & 6 deletions pkg/kv/kvserver/concurrency/lock_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1179,16 +1179,35 @@ func (l *lockState) acquireLock(
// - txn A's coordinator does not immediately learn of the push
// - txn A re-acquires lock at sequence 2, ts 15
//
// A lock's timestamp cannot be allowed to regress, so by forwarding
// its timestamp during the second acquisition instead if assigning
// to it blindly, it remains at 20.
// A lock's timestamp at a given durability level is not allowed to
// regress, so by forwarding its timestamp during the second acquisition
// instead if assigning to it blindly, it remains at 20.
//
// However, a lock's timestamp as reported by getLockerInfo can regress
// if it is acquired at a lower timestamp and a different durability
// than it was previously held with. This is necessary to support
// because the hard constraint which we must uphold here that the
// lockHolderInfo for a replicated lock cannot diverge from the
// replicated state machine in such a way that its timestamp in the
// lockTable exceeds that in the replicated keyspace. If this invariant
// were to be violated, we'd risk infinite lock-discovery loops for
// requests that conflict with the lock as is written in the replicated
// state machine but not as is reflected in the lockTable.
//
// Lock timestamp regressions are safe from the perspective of other
// transactions because the request which re-acquired the lock at the
// lower timestamp must have been holding a write latch at or below the
// new lock's timestamp. This means that no conflicting requests could
// be evaluating concurrently. Instead, all will need to re-scan the
// lockTable once they acquire latches and will notice the reduced
// timestamp at that point, which may cause them to conflict with the
// lock even if they had not conflicted before. In a sense, it is no
// different than the first time a lock is added to the lockTable.
l.holder.holder[durability].ts.Forward(ts)
l.holder.holder[durability].seqs = append(seqs, txn.Sequence)

_, afterTs, _ := l.getLockerInfo()
if afterTs.Less(beforeTs) {
panic("lockTable bug - lock timestamp regression")
} else if beforeTs.Less(afterTs) {
if beforeTs.Less(afterTs) {
l.increasedLockTs(afterTs)
}
return nil
Expand Down
128 changes: 126 additions & 2 deletions pkg/kv/kvserver/concurrency/testdata/concurrency_manager/update
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# still at the original timestamp. This is permitted but the
# lock's timestamp should not regress.
#
# Setup: txn1 acquire lock k
# Setup: txn1 acquires lock k
# txn2 reads k and waits
# txn2 pushes txn1
#
Expand Down Expand Up @@ -124,7 +124,7 @@ reset namespace
# the original timestamp. This is permitted but the lock's
# timestamp should not regress.
#
# Setup: txn1 acquire lock k
# Setup: txn1 acquires lock k
# txn2 reads k and waits
#
# Test: txn2 pushes txn1's timestamp forward
Expand Down Expand Up @@ -238,3 +238,127 @@ local: num=0

reset namespace
----

# -------------------------------------------------------------
# A transaction acquires an unreplicated lock. The lock is
# pushed to a higher timestamp by a second transaction. The
# transaction then returns to upgrade the unreplicated lock to a
# replicated intent at a new sequence number but still at the
# original timestamp. This is permitted and the lock's timestamp
# regresses back down to the intent's timestamp. In practice, if
# the pusher wanted to prevent its push from being reverted, it
# should have also bumped the timestamp cache to ensure that the
# intent couldn't be laid down at the original timestamp.
#
# Setup: txn1 acquires unreplicated lock k
# txn2 reads k and waits
# txn2 pushes txn1
#
# Test: txn2 succeeds in pushing txn1's ts forward
# txn2 proceeds
# txn1 re-acquires replicated lock k at lower ts
# -------------------------------------------------------------

new-txn name=txn1 ts=10,1 epoch=0
----

new-txn name=txn2 ts=12,1 epoch=0
----

new-request name=req1 txn=txn1 ts=10,1
put key=k value=v
----

new-request name=req2 txn=txn2 ts=12,1
get key=k
----

sequence req=req1
----
[1] sequence req1: sequencing request
[1] sequence req1: acquiring latches
[1] sequence req1: scanning lock table for conflicting locks
[1] sequence req1: sequencing complete, returned guard

on-lock-acquired req=req1 key=k dur=u
----
[-] acquire lock: txn 00000001 @ k

finish req=req1
----
[-] finish req1: finishing request

sequence req=req2
----
[2] sequence req2: sequencing request
[2] sequence req2: acquiring latches
[2] sequence req2: scanning lock table for conflicting locks
[2] sequence req2: waiting in lock wait-queues
[2] sequence req2: pushing timestamp of txn 00000001 above 0.000000012,1
[2] sequence req2: blocked on select in concurrency_test.(*cluster).PushTransaction

debug-lock-table
----
global: num=1
lock: "k"
holder: txn: 00000001-0000-0000-0000-000000000000, ts: 0.000000010,1, info: unrepl epoch: 0, seqs: [0]
waiting readers:
req: 8, txn: 00000002-0000-0000-0000-000000000000
distinguished req: 8
local: num=0

# --------------------------------
# Setup complete, test starts here
# --------------------------------

on-txn-updated txn=txn1 status=pending ts=12,2
----
[-] update txn: increasing timestamp of txn1
[2] sequence req2: resolving intent "k" for txn 00000001 with PENDING status
[2] sequence req2: acquiring latches
[2] sequence req2: scanning lock table for conflicting locks
[2] sequence req2: sequencing complete, returned guard

finish req=req2
----
[-] finish req2: finishing request

debug-lock-table
----
global: num=1
lock: "k"
holder: txn: 00000001-0000-0000-0000-000000000000, ts: 0.000000012,2, info: unrepl epoch: 0, seqs: [0]
local: num=0

# Issue another write to the same key for txn1 at its initial timestamp,
# this time with a replicated durability. The timestamp in the lock
# table should regress back down to reflect the replicated lock state.

new-request name=req3 txn=txn1 ts=10,1
put key=k value=v2 seq=1
----

sequence req=req3
----
[3] sequence req3: sequencing request
[3] sequence req3: acquiring latches
[3] sequence req3: scanning lock table for conflicting locks
[3] sequence req3: sequencing complete, returned guard

on-lock-acquired req=req3 key=k seq=1 dur=r
----
[-] acquire lock: txn 00000001 @ k

finish req=req3
----
[-] finish req3: finishing request

debug-lock-table
----
global: num=1
lock: "k"
holder: txn: 00000001-0000-0000-0000-000000000000, ts: 0.000000010,1, info: repl epoch: 0, seqs: [1], unrepl epoch: 0, seqs: [0]
local: num=0

reset namespace
----
41 changes: 34 additions & 7 deletions pkg/kv/kvserver/concurrency/testdata/lock_table/lock_changes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ global: num=1
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000010,0, info: unrepl epoch: 0, seqs: [2, 3]
local: num=0

# ---------------------------------------------------------------------------------
# Lock is reacquired at same epoch with lower timestamp and different durability.
# The lock's timestamp is allowed to regress in this case, because it must never
# diverge from the replicated state machine.
# ---------------------------------------------------------------------------------

acquire r=req2 k=a durability=r
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000008,0, info: repl epoch: 0, seqs: [3], unrepl epoch: 0, seqs: [2, 3]
local: num=0

# ---------------------------------------------------------------------------------
# Lock is reacquired at a different epoch. The old sequence numbers are discarded.
# ---------------------------------------------------------------------------------
Expand All @@ -63,7 +76,7 @@ acquire r=req3 k=a durability=u
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000010,0, info: unrepl epoch: 1, seqs: [0]
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000008,0, info: repl epoch: 0, seqs: [3], unrepl epoch: 1, seqs: [0]
local: num=0

# ---------------------------------------------------------------------------------
Expand All @@ -72,17 +85,17 @@ local: num=0
# discarded but the timestamp is not regressed.
# ---------------------------------------------------------------------------------

new-txn txn=txn1 ts=8 epoch=2 seq=0
new-txn txn=txn1 ts=6 epoch=2 seq=0
----

new-request r=req4 txn=txn1 ts=8 spans=w@a
new-request r=req4 txn=txn1 ts=6 spans=w@a
----

acquire r=req4 k=a durability=u
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000010,0, info: unrepl epoch: 2, seqs: [0]
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000008,0, info: repl epoch: 0, seqs: [3], unrepl epoch: 2, seqs: [0]
local: num=0

# ---------------------------------------------------------------------------------
Expand All @@ -98,13 +111,13 @@ start-waiting: true

guard-state r=req5
----
new: state=waitForDistinguished txn=txn1 ts=10 key="a" held=true guard-access=read
new: state=waitForDistinguished txn=txn1 ts=8 key="a" held=true guard-access=read

print
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000010,0, info: unrepl epoch: 2, seqs: [0]
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000008,0, info: repl epoch: 0, seqs: [3], unrepl epoch: 2, seqs: [0]
waiting readers:
req: 1, txn: 00000000-0000-0000-0000-000000000002
distinguished req: 1
Expand All @@ -116,11 +129,25 @@ new-txn txn=txn1 ts=14 epoch=1 seq=1
new-request r=req6 txn=txn1 ts=14 spans=w@a
----

acquire r=req6 k=a durability=r
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000010,0, info: repl epoch: 1, seqs: [1], unrepl epoch: 2, seqs: [0]
waiting readers:
req: 1, txn: 00000000-0000-0000-0000-000000000002
distinguished req: 1
local: num=0

guard-state r=req5
----
old: state=waitForDistinguished txn=txn1 ts=8 key="a" held=true guard-access=read

acquire r=req6 k=a durability=u
----
global: num=1
lock: "a"
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000014,0, info: unrepl epoch: 1, seqs: [0, 1]
holder: txn: 00000000-0000-0000-0000-000000000001, ts: 0.000000014,0, info: repl epoch: 1, seqs: [1], unrepl epoch: 1, seqs: [0, 1]
local: num=0

guard-state r=req5
Expand Down