pessimistic-transaction: clarify Point Get lock behavior for non-existing keys under different isolation levels#23111
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request adds a note to the pessimistic transactions documentation clarifying that Point Get and Batch Point Get operators only lock non-existing keys under the REPEATABLE READ isolation level, and not under READ COMMITTED. The reviewer provided a helpful suggestion to improve the clarity and phrasing of this note by using more concise and idiomatic language.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
96cec23 to
99c177b
Compare
99c177b to
c857e57
Compare
Verification ScriptI verified the behavior described in this PR with the following test script: #!/usr/bin/env bash
# Test script to validate Point Get / Batch Point Get lock behavior
# for non-existing keys under different isolation levels in TiDB.
#
# Usage:
# ./scripts/test-pessimistic-point-get-lock.sh <mysql_host> <mysql_port> <user> <password>
#
# Prerequisites:
# - mysql CLI client installed
# - A running TiDB cluster with pessimistic transaction mode enabled
set -euo pipefail
HOST=${1:-127.0.0.1}
PORT=${2:-4000}
USER=${3:-root}
PASS=${4:-}
MYSQL="mysql -h${HOST} -P${PORT} -u${USER}"
if [ -n "${PASS}" ]; then
MYSQL="${MYSQL} -p${PASS}"
fi
echo "=== Setting up test table ==="
${MYSQL} -e "DROP TABLE IF EXISTS test.t; CREATE TABLE test.t (id INT PRIMARY KEY, v INT);"
echo ""
echo "=== Test 1: Repeatable Read (default) + Point Get on non-existing key ==="
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 999 FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 999 FOR UPDATE;" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (999, 99); (expected: blocked -> timeout)"
timeout 5 ${MYSQL} -e "INSERT INTO test.t VALUES (999, 99);" || echo "Session 2: Blocked as expected (Lock wait timeout)"
echo "Session 1: ROLLBACK;"
${MYSQL} -e "ROLLBACK;" 2>/dev/null || true
wait $SESS1_PID 2>/dev/null || true
echo ""
echo "=== Test 2: Read Committed + Point Get on non-existing key ==="
${MYSQL} -e "DELETE FROM test.t WHERE id = 888;"
echo "Session 1: SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED; BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 888 FOR UPDATE;"
${MYSQL} -e "SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED; BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 888 FOR UPDATE;" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (888, 88); (expected: SUCCESS, not blocked)"
${MYSQL} -e "INSERT INTO test.t VALUES (888, 88);" && echo "Session 2: Insert succeeded as expected (no lock in RC)"
echo "Session 1: ROLLBACK;"
${MYSQL} -e "ROLLBACK;" 2>/dev/null || true
wait $SESS1_PID 2>/dev/null || true
echo ""
echo "=== Test 3: Repeatable Read + Batch Point Get on non-existing keys ==="
${MYSQL} -e "DELETE FROM test.t WHERE id IN (777, 888);"
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id IN (777, 888) FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id IN (777, 888) FOR UPDATE;" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (777, 777); (expected: blocked -> timeout)"
timeout 5 ${MYSQL} -e "INSERT INTO test.t VALUES (777, 777);" || echo "Session 2: Blocked as expected (Lock wait timeout)"
echo "Session 1: ROLLBACK;"
${MYSQL} -e "ROLLBACK;" 2>/dev/null || true
wait $SESS1_PID 2>/dev/null || true
echo ""
echo "=== Test 4: Range scan (no gap lock in TiDB) ==="
${MYSQL} -e "DELETE FROM test.t WHERE id BETWEEN 100 AND 200;"
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id BETWEEN 100 AND 200 FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id BETWEEN 100 AND 200 FOR UPDATE;" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (150, 150); (expected: SUCCESS, not blocked)"
${MYSQL} -e "INSERT INTO test.t VALUES (150, 150);" && echo "Session 2: Insert succeeded as expected (no gap lock)"
echo "Session 1: ROLLBACK;"
${MYSQL} -e "ROLLBACK;" 2>/dev/null || true
wait $SESS1_PID 2>/dev/null || true
echo ""
echo "=== Cleanup ==="
${MYSQL} -e "DROP TABLE IF EXISTS test.t;"
echo ""
echo "=== All tests completed ==="Verified Results
|
Updated Verification Results (Tested on TiDB v8.5.3)
Key finding: Under |
c857e57 to
fc3e6cc
Compare
|
@ekexium: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
qiancai
left a comment
There was a problem hiding this comment.
The documentation statement is consistent with the TiDB v8.5 TestLockGotKeysInRC regression test, which covers Point Get and Batch Point Get on both primary and unique keys. However, the verification script posted in the PR does not keep Session 1 open: mysql -e "BEGIN ...; SELECT ... FOR UPDATE;" exits after executing the statements, so the connection closes and the transaction is rolled back before the script sleeps and starts Session 2. The later ROLLBACK also runs in a new connection and therefore cannot roll back Session 1.
Could you revise the script to keep two persistent client sessions open (or use a small program with two retained connections)? This does not invalidate the documentation change, but the current script cannot independently reproduce the reported lock-wait results.
[LGTM Timeline notifier]Timeline:
|
Co-authored-by: Grace Cai <qqzczy@126.com>
Corrected Verification ScriptThe original script used #!/usr/bin/env bash
# Test script to validate Point Get / Batch Point Get lock behavior
# for non-existing keys under different isolation levels in TiDB.
#
# Usage:
# ./test-pessimistic-point-get-lock.sh <mysql_host> <mysql_port> <user> <password>
#
# Prerequisites:
# - mysql CLI client installed
# - A running TiDB cluster with pessimistic transaction mode enabled
set -euo pipefail
HOST=${1:-127.0.0.1}
PORT=${2:-4000}
USER=${3:-root}
PASS=${4:-}
MYSQL="mysql -h${HOST} -P${PORT} -u${USER}"
if [ -n "${PASS}" ]; then
MYSQL="${MYSQL} -p${PASS}"
fi
cleanup() {
kill $SESS1_PID 2>/dev/null || true
wait $SESS1_PID 2>/dev/null || true
}
trap cleanup EXIT
echo "=== Setting up test table ==="
${MYSQL} -e "DROP TABLE IF EXISTS test.t; CREATE TABLE test.t (id INT PRIMARY KEY, v INT);"
echo ""
echo "=== Test 1: Repeatable Read (default) + Point Get on non-existing key ==="
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 999 FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 999 FOR UPDATE; SELECT SLEEP(15);" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (999, 99); (expected: blocked -> timeout)"
timeout 5 ${MYSQL} -e "INSERT INTO test.t VALUES (999, 99);" || echo "Session 2: Blocked as expected (Lock wait timeout)"
cleanup
echo ""
echo "=== Test 2: Read Committed + Point Get on non-existing key ==="
${MYSQL} -e "DELETE FROM test.t WHERE id = 888;"
echo "Session 1: SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED; BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 888 FOR UPDATE;"
${MYSQL} -e "SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED; BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id = 888 FOR UPDATE; SELECT SLEEP(15);" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (888, 88); (expected: SUCCESS, not blocked)"
${MYSQL} -e "INSERT INTO test.t VALUES (888, 88);" && echo "Session 2: Insert succeeded as expected (no lock in RC)"
cleanup
echo ""
echo "=== Test 3: Repeatable Read + Batch Point Get on non-existing keys ==="
${MYSQL} -e "DELETE FROM test.t WHERE id IN (777, 888);"
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id IN (777, 888) FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id IN (777, 888) FOR UPDATE; SELECT SLEEP(15);" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (777, 777); (expected: blocked -> timeout)"
timeout 5 ${MYSQL} -e "INSERT INTO test.t VALUES (777, 777);" || echo "Session 2: Blocked as expected (Lock wait timeout)"
cleanup
echo ""
echo "=== Test 4: Range scan (no gap lock in TiDB) ==="
${MYSQL} -e "DELETE FROM test.t WHERE id BETWEEN 100 AND 200;"
echo "Session 1: BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id BETWEEN 100 AND 200 FOR UPDATE;"
${MYSQL} -e "BEGIN PESSIMISTIC; SELECT * FROM test.t WHERE id BETWEEN 100 AND 200 FOR UPDATE; SELECT SLEEP(15);" &
SESS1_PID=$!
sleep 1
echo "Session 2: INSERT INTO test.t VALUES (150, 150); (expected: SUCCESS, no gap lock)"
${MYSQL} -e "INSERT INTO test.t VALUES (150, 150);" && echo "Session 2: Insert succeeded (no gap lock in TiDB)"
cleanup
echo ""
echo "=== Cleanup ==="
${MYSQL} -e "DROP TABLE IF EXISTS test.t;"
echo ""
echo "=== All tests completed ===" |
Verification Results (corrected script)Ran the corrected script against TiDB playground:
All results match the documented behavior. The |
What is changed, added or deleted? (Required)
Added a Note in the
Behaviorssection ofpessimistic-transaction.mdto clarify that the Point Get and Batch Point Get lock behavior for non-existing keys applies only to theREPEATABLE READisolation level.Under the
READ COMMITTEDisolation level,Point GetandBatch Point Getoperators do not lock non-existent keys. This is a behavioral difference that can affect applications usingSELECT FOR UPDATEfor existence checks followed byINSERT.Which TiDB version(s) do your changes apply to? (Required)
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?
Verification
This behavior was verified on TiDB v8.5.3:
id = 999id = 888id IN (777, 888)id BETWEEN 100 AND 200A test script was provided in the PR comments for reviewers to verify the behavior locally.