Merge #145220

145220: logictest: harden a couple buffered writes tests r=yuzefovich a=yuzefovich

This commit fixes two flakes in `buffered_writes` logic test:
- the test added in 31ab5c3 can result in an extra Scan request in the trace with low `default-batch-bytes-limit` value (we paginate over several rows). I don't think we need to use tracing as a regression test for checking ReturnRawMVCCValues option, so this commit simply uses the `crdb_internal_origin_id` explicitly and deletes the tracing tests
- the test added in 238f0e3 has a txn that writes some rows and then reads large already present blobs. We've seen a few cases when this hits a RETRY_SERIALIZABLE error under race. My guess is that it could happen if the txn becomes long-running due to race config overhead, especially in fakedist configs, so this commit bumps the closed ts interval to avoid hitting the retry error.

Additionally this commit fixes a couple of nits.

Fixes: #144756.
Fixes: #145065.
Fixes: #145280.
Fixes: #145281.
Fixes: #145282.
Fixes: #145285.
Fixes: #145286.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/kv/kvclient/kvcoord/txn_interceptor_write_buffer.go

@@ -356,13 +356,13 @@ func (twb *txnWriteBuffer) validateRequests(ba *kvpb.BatchRequest) error {
 			// ReturnRawMVCCValues is unsupported because we don't know how to serve
 			// such reads from the write buffer currently.
 			if t.ReturnRawMVCCValues {
-				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValue")
+				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValues")
 			}
 		case *kvpb.ScanRequest:
 			// ReturnRawMVCCValues is unsupported because we don't know how to serve
 			// such reads from the write buffer currently.
 			if t.ReturnRawMVCCValues {
-				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValue")
+				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValues")
 			}
 			if t.ScanFormat == kvpb.COL_BATCH_RESPONSE {
 				return unsupportedOptionError(t.Method(), "COL_BATCH_RESPONSE scan format")
@@ -371,7 +371,7 @@ func (twb *txnWriteBuffer) validateRequests(ba *kvpb.BatchRequest) error {
 			// ReturnRawMVCCValues is unsupported because we don't know how to serve
 			// such reads from the write buffer currently.
 			if t.ReturnRawMVCCValues {
-				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValue")
+				return unsupportedOptionError(t.Method(), "ReturnRawMVCCValues")
 			}
 			if t.ScanFormat == kvpb.COL_BATCH_RESPONSE {
 				return unsupportedOptionError(t.Method(), "COL_BATCH_RESPONSE scan format")

pkg/sql/delete_range.go

@@ -181,10 +181,6 @@ func (d *deleteRangeNode) deleteSpans(params runParams, b *kv.Batch, spans roach
 			if traceKV {
 				log.VEventf(ctx, 2, "DelRange %s - %s", span.Key, span.EndKey)
 			}
-			// TODO(yuzefovich): decide what we do with DeleteRange requests. If
-			// we won't buffer them, then we don't need to make any changes; if
-			// we do buffer them in the interceptor, we'll need to set
-			// to-be-added MustAcquireExclusiveLock flag too.
 			b.DelRange(span.Key, span.EndKey, true /* returnKeys */)
 		}
 	}

pkg/sql/logictest/testdata/logic_test/buffered_writes

@@ -261,64 +261,19 @@ EXPLAIN CREATE DATABASE foo
 statement ok
 EXPLAIN ANALYZE CREATE DATABASE foo
 
-# Regression test for #144273.
-statement ok
-CREATE TABLE t144273 (
-  k INT PRIMARY KEY,
-  a INT,
-  b INT,
-  INDEX (a),
-  INDEX (b),
-  FAMILY (k, a, b)
-)
-
-statement ok
-PREPARE p AS UPDATE t144273 t1 SET a = t2.a + 1, b = t2.b + 1 FROM t144273 t2 WHERE t1.k = t2.a AND t1.k = $1
+# Regression test for #144273 (not supporting ReturnRawMVCCValues option).
 
+# Get request.
 statement ok
-SET vectorize = on
+SELECT k, crdb_internal_origin_id FROM t4 WHERE k = 1
 
+# Scan request.
 statement ok
-INSERT INTO t144273 VALUES (1, 1, 1);
-
-# The table IDs in the kv trace below are different with the legacy schema
-# changer, so disable that configuration.
-skipif config local-legacy-schema-changer
-query T kvtrace
-EXECUTE p(1)
-----
-Scan /Table/114/2/{1-2}
-Scan /Table/114/1/1/0
-Scan /Table/114/1/1/0
-Put (locking) /Table/114/1/1/0 -> /TUPLE/2:2:Int/2/1:3:Int/2
-Del /Table/114/2/1/1/0
-Put /Table/114/2/2/1/0 -> /BYTES/
-Del /Table/114/3/1/1/0
-Put /Table/114/3/2/1/0 -> /BYTES/
+SELECT k, crdb_internal_origin_id FROM t4
 
+# ReverseScan request.
 statement ok
-SET vectorize = off
-
-statement ok
-INSERT INTO t144273 VALUES (2, 2, 2);
-
-# The table IDs in the kv trace below are different with the legacy schema
-# changer, so disable that configuration.
-skipif config local-legacy-schema-changer
-query T kvtrace
-EXECUTE p(2)
-----
-Scan /Table/114/2/{2-3}
-Scan /Table/114/1/1/0, /Table/114/1/2/0
-Scan /Table/114/1/2/0
-Put (locking) /Table/114/1/2/0 -> /TUPLE/2:2:Int/3/1:3:Int/3
-Del /Table/114/2/2/2/0
-Put /Table/114/2/3/2/0 -> /BYTES/
-Del /Table/114/3/2/2/0
-Put /Table/114/3/3/2/0 -> /BYTES/
-
-statement ok
-RESET vectorize
+SELECT k, crdb_internal_origin_id FROM t4 ORDER BY k DESC
 
 statement ok
 CREATE TABLE uvw (
@@ -440,6 +395,19 @@ SELECT count(*) FROM uvw
 ----
 64
 
+# Increase the closed timestamp interval since the transaction below can take
+# long time under race which could result in a RETRY_SERIALIZABLE error on
+# COMMIT (#145065).
+#
+# The cluster setting is SystemVisible, so we must run this query on the host
+# cluster in multi-tenant environment.
+user host-cluster-root
+
+statement ok
+SET CLUSTER SETTING kv.closed_timestamp.target_duration = '30s'
+
+user root
+
 # Create some large blobs that will require the scans to be paginated (in 10MiB
 # chunks).
 statement ok