Merge #25014 #25791 #26293

25014: storage: queue requests to push txn / resolve intents on single keys r=spencerkimball a=spencerkimball

Previously, high contention on a single key would cause every thread to
push the same conflicting transaction then resolve the same intent in
parallel. This is inefficient as only one pusher needs to succeed, and
only one resolver needs to resolve the intent, and then only one writer
should proceed while the other readers/writers should in turn wait on
the previous writer by pushing its transaction. This effectively
serializes the conflicting reader/writers.
    
One complication is that all pushers which may have a valid, writing
transaction (i.e., `Transaction.Key != nil`), must push either the
conflicting transaction or another transaction already pushing that
transaction. This allows dependency cycles to be discovered.

Fixes #20448 

25791: jobs: bump default progress log time to 30s r=mjibson a=mjibson

The previous code allowed updates to be performed every 1s, which could
cause the MVCC row to be very large causing problems with splits. We
can update much more slowly by default. In the case of a small backup
job, the 5% fraction threshold will allow a speedier update rate.

Remove a note that's not useful anymore since the referred function
can now only be used in the described safe way.

See #25770. Although this change didn't fix that bug, we still think
it's a good idea.

Release note: None

26293: opt: enable a few distsql logictests r=RaduBerinde a=RaduBerinde

 - `distsql_indexjoin`: this is only a planning test. Modifying the
   split points and queries a bit to make the condition more
   restrictive and make the optimizer choose index joins. There was a
   single plan that was different, and the difference was minor (the
   old planner is emitting an unnecessary column).

 - `distsql_expr`: logic-only test, enabling for opt.

 - `distsql_scrub`: planning test; opt version commented out for now.

Release note: None

Co-authored-by: Spencer Kimball <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>

pkg/sql/jobs/progress.go

@@ -23,7 +23,7 @@ import (
 // last logged fractionCompleted and the current fractionCompleted is more than
 // progressFractionThreshold.
 var (
-	progressTimeThreshold             = time.Second
+	progressTimeThreshold             = 30 * time.Second
 	progressFractionThreshold float32 = 0.05
 )
 
@@ -53,14 +53,6 @@ type ProgressLogger struct {
 // chunkFinished marks one chunk of the job as completed. If either the time or
 // fraction threshold has been reached, the progress update will be persisted to
 // system.jobs.
-//
-// NB: chunkFinished is not threadsafe. A previous implementation that was
-// threadsafe occasionally led to massive contention. One 2TB restore on a 15
-// node cluster, for example, had 60 goroutines attempting to update the
-// progress at once, causing massive contention on the row in system.jobs. This
-// inadvertently applied backpressure on the restore's import requests and
-// slowed the job to a crawl. If multiple threads need to update progress, use a
-// channel and a dedicated goroutine that calls loop.
 func (jpl *ProgressLogger) chunkFinished(ctx context.Context) error {
 	jpl.completedChunks++
 	fraction := float32(jpl.completedChunks) / float32(jpl.TotalChunks)

pkg/sql/logictest/testdata/logic_test/distsql_expr

@@ -1,4 +1,4 @@
-# LogicTest: default distsql distsql-metadata
+# LogicTest: default opt distsql distsql-opt distsql-metadata
 
 statement ok
 CREATE TABLE t (c int PRIMARY KEY)
@@ -12,6 +12,3 @@ SELECT c FROM t WHERE (c, c) > (2, -9223372036854775808)
 ----
 2
 3
-
-statement ok
-CREATE TABLE ab (a int PRIMARY KEY, b int)

pkg/sql/logictest/testdata/logic_test/distsql_indexjoin → pkg/sql/logictest/testdata/planner_test/distsql_indexjoin

@@ -1,45 +1,45 @@
-# LogicTest: 5node-distsql 5node-distsql-metadata
+# LogicTest: 5node-distsql
 
 statement ok
 CREATE TABLE t (k INT PRIMARY KEY, v INT, w INT, INDEX v(v))
 
-# Split the index into 5 parts. We expect numbers in the range 1 to 1000.
+# Split the index into 5 parts, as if numbers were in the range 1 to 100.
 statement ok
-ALTER INDEX t@v SPLIT AT SELECT (i * 100)::int FROM GENERATE_SERIES(1, 4) AS g(i)
+ALTER INDEX t@v SPLIT AT SELECT (i * 10)::int FROM GENERATE_SERIES(1, 4) AS g(i)
 
 # Relocate the five parts to the five nodes.
 statement ok
 ALTER INDEX t@v TESTING_RELOCATE
-  SELECT ARRAY[i+1], (i * 100)::int FROM GENERATE_SERIES(0, 4) AS g(i)
+  SELECT ARRAY[i+1], (i * 10)::int FROM GENERATE_SERIES(0, 4) AS g(i)
 
 query TTITI colnames
 SHOW TESTING_RANGES FROM INDEX t@v
 ----
 Start Key  End Key  Range ID  Replicas  Lease Holder
-NULL       /100     1         {1}       1
-/100       /200     2         {2}       2
-/200       /300     3         {3}       3
-/300       /400     4         {4}       4
-/400       NULL     5         {5}       5
+NULL       /10      1         {1}       1
+/10        /20      2         {2}       2
+/20        /30      3         {3}       3
+/30        /40      4         {4}       4
+/40        NULL     5         {5}       5
 
 query T
-SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 100]
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 10 AND v < 50]
 ----
 https://cockroachdb.github.io/distsqlplan/decode.html?eJyslDtrwzAURvf-ivLNt8SvPOrJazokJXQrHlzrEgyJZSS5tJT89-IoUDk0ioMz6nHu8RmsH9RS8KrYs0b6jhCECIQYhASEKXJCo2TJWkvVXbHAUnwhDQhV3bTGbpvK7Bgp2loqwYoFCIJNUe268_yQE0qpGOnf1ZV8ks1kcXaRIFtzGpsTtCm2jDQ-kKMOHfU_g9-Kjx1vuBCsJkFvPD4zA8K6NeljFuKSLbzF9iKr-iRL-rJGVftCfbtKyiLK4oviqCeOhmeG4zOv2JzM6X0z4-GZ0fjMKzYnc3bfzGR4Zjw-84rNyZzfNzPwizesG1lrHvTXB92zwWLL9o3RslUlvypZHjV2uT5yxw3B2tjTZ7tY1vao-0AXDr1w5IcjLxz04PAcjr1w4jcnY8xTLzzzm2djzHMvvPCbFzeZ88PDbwAAAP__eQw40w==
 
 query T
-SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 100 ORDER BY v]
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 10 AND v < 50 ORDER BY v]
 ----
 https://cockroachdb.github.io/distsqlplan/decode.html?eJyslM1uozAURvfzFKNvO3cUMORnWLHNLJIq6q5iQfFVhJRgZJuqVcW7V8GRClHjEIWlf46Pz-Z-olKSN_mRDZIXhCAIECIQYhDmyAi1VgUbo_TpigPW8h1JQCirurFu25b2wEigtGTNEgTJNi8P3bup-IOszQiF0ozk-_ZG_VX1bDW4nbUE1djzyxnB2HzPSKKWevawZ__h4ef89cA7ziXrWTD8zFtqQdg2NvmdhpQKXBOG9wj_q7I6--Khr9blMdcfF1ZKo6tiMRCL8aXhJKU3hL3S-bSl0fhSMUnpDWGvdDFtaTy-NJqk9IawV7qctjTwi3dsalUZHjUBgtMIYblnN3KManTBT1oVncYttx3XbUg21p3-c4t15Y5OH-zDoRcWflh44WAAh5dw5IVjvzl-xDz3wgu_efGIeemFV37z6i5z1v76CgAA___W2DtJ
 
 # Here we care about ordering by v, but v is not otherwise used.
 query T
-SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 100 ORDER BY v]
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 10 AND v < 50 ORDER BY v]
 ----
 https://cockroachdb.github.io/distsqlplan/decode.html?eJyslL9uqzAYR_f7FFe_ta6CDflTJtZ0SKqoW8VA8acIKcHINlWrinevgiPVVK1DFEZsn-_4DPgTtZK0KY5kkL6Ag0GAIQZDAoY5coZGq5KMUfp0xAFr-Y40YqjqprVu2Vb2QEihtCRNEgySbFEd-rmZuEPe5Qyl0oT0-_RG3atmthqczjsG1drz5JzB2GJPSOOOeXbu2X8Z_Fy8HmhHhSQ9i4aXecssGLatTf9nnGUCfwn5NcJHVdVnXzL0Nbo6FvrDs8YhqxhYxfhMPknmBaGXOZ8wMx6fKSbJvCD0MhcTZibjM-NJMi8IvczlhJlR2Loj06ja0Ki_Pjo9GyT35J4Zo1pd0pNWZa9xn9ue6xckGet2H9zHunZbpwv6MA_CIgyLIBwNYP4TjoNwEjYnt5jnQXgRNi9uMS-D8CpsXl1lzrt_XwEAAP__NMo41Q==
 
 # The single join reader should be on node 5, and doesn't need to output v.
 query T
-SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 400 ORDER BY v]
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 40 AND v < 50 ORDER BY v]
 ----
 https://cockroachdb.github.io/distsqlplan/decode.html?eJyUkT1rwzAQhvf-ivLOKrFsumjKmg5JCd2KB9U6gsHRCelcWoL_e7FVqF2I24z38byPOF3g2dHeninBvEJD4RG1QojcUEocx3Ze2rkPmEKh9aGXsV0rNBwJ5gJppSMY7PmBw6aEgiOxbTetDQrcyw-UxJ4IphrULFivB7_Yt46OZB3FTbGIx_tWoHDoxdxvNa7Z9C22J279t0wvZSG2Zxs_Z8rqqrJcKP-43JFSYJ_oX6crhlqB3Iny7yTuY0PPkZtJk8vDxE0NR0nytMrFzufR-MA5rFfhch0uV-HiF1wPd18BAAD__56h0KE=

pkg/sql/logictest/testdata/logic_test/distsql_scrub → pkg/sql/logictest/testdata/planner_test/distsql_scrub

@@ -1,4 +1,4 @@
-# LogicTest: 5node-distsql 5node-distsql-metadata 5node-distsql-disk
+# LogicTest: 5node-distsql
 
 # Verify the index check execution plan uses a merge join.
 

pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin

@@ -0,0 +1,45 @@
+# LogicTest: 5node-distsql-opt
+
+statement ok
+CREATE TABLE t (k INT PRIMARY KEY, v INT, w INT, INDEX v(v))
+
+# Split the index into 5 parts, as if numbers were in the range 1 to 100.
+statement ok
+ALTER INDEX t@v SPLIT AT SELECT (i * 10)::int FROM GENERATE_SERIES(1, 4) AS g(i)
+
+# Relocate the five parts to the five nodes.
+statement ok
+ALTER INDEX t@v TESTING_RELOCATE
+  SELECT ARRAY[i+1], (i * 10)::int FROM GENERATE_SERIES(0, 4) AS g(i)
+
+query TTITI colnames
+SHOW TESTING_RANGES FROM INDEX t@v
+----
+Start Key  End Key  Range ID  Replicas  Lease Holder
+NULL       /10      1         {1}       1
+/10        /20      2         {2}       2
+/20        /30      3         {3}       3
+/30        /40      4         {4}       4
+/40        NULL     5         {5}       5
+
+query T
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 10 AND v < 50]
+----
+https://cockroachdb.github.io/distsqlplan/decode.html?eJyslDtrwzAURvf-ivLNt8SvPOrJazokJXQrHlzrEgyJZSS5tJT89-IoUDk0ioMz6nHu8RmsH9RS8KrYs0b6jhCECIQYhASEKXJCo2TJWkvVXbHAUnwhDQhV3bTGbpvK7Bgp2loqwYoFCIJNUe268_yQE0qpGOnf1ZV8ks1kcXaRIFtzGpsTtCm2jDQ-kKMOHfU_g9-Kjx1vuBCsJkFvPD4zA8K6NeljFuKSLbzF9iKr-iRL-rJGVftCfbtKyiLK4oviqCeOhmeG4zOv2JzM6X0z4-GZ0fjMKzYnc3bfzGR4Zjw-84rNyZzfNzPwizesG1lrHvTXB92zwWLL9o3RslUlvypZHjV2uT5yxw3B2tjTZ7tY1vao-0AXDr1w5IcjLxz04PAcjr1w4jcnY8xTLzzzm2djzHMvvPCbFzeZ88PDbwAAAP__eQw40w==
+
+query T
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT * FROM t WHERE v > 10 AND v < 50 ORDER BY v]
+----
+https://cockroachdb.github.io/distsqlplan/decode.html?eJyslM1uozAURvfzFKNvO3cUMORnWLHNLJIq6q5iQfFVhJRgZJuqVcW7V8GRClHjEIWlf46Pz-Z-olKSN_mRDZIXhCAIECIQYhDmyAi1VgUbo_TpigPW8h1JQCirurFu25b2wEigtGTNEgTJNi8P3bup-IOszQiF0ozk-_ZG_VX1bDW4nbUE1djzyxnB2HzPSKKWevawZ__h4ef89cA7ziXrWTD8zFtqQdg2NvmdhpQKXBOG9wj_q7I6--Khr9blMdcfF1ZKo6tiMRCL8aXhJKU3hL3S-bSl0fhSMUnpDWGvdDFtaTy-NJqk9IawV7qctjTwi3dsalUZHjUBgtMIYblnN3KManTBT1oVncYttx3XbUg21p3-c4t15Y5OH-zDoRcWflh44WAAh5dw5IVjvzl-xDz3wgu_efGIeemFV37z6i5z1v76CgAA___W2DtJ
+
+# Here we care about ordering by v, but v is not otherwise used.
+query T
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 10 AND v < 50 ORDER BY v]
+----
+https://cockroachdb.github.io/distsqlplan/decode.html?eJyslL9uqzAYR_f7FFe_9foq2JA_ZWJNh6SKulUMFH-KkBKMbFO1qnj3KjhSTdU4RGHE9vmOz4A_UStJm-JIBukLOBgEGGIwJGCYI2dotCrJGKVPRxywlu9II4aqblrrlm1lD4QUSkvSJMEgyRbVoZ-b8X_Iu5yhVJqQfp_eqP-qma0Gp_OOQbX2PDlnMLbYE9K4Y56de_ZfBj8XrwfaUSFJz6LhZd4yC4Zta9O_GWeZwCUhv0X4qKr67EuGvkZXx0J_eNb4olIMlGJ8I5-k8YrQa5xP1RiPbxSTNF4Reo2LqRqT8Y3xJI1XhF7jcqrGKKzckWlUbWjUnx6dngqSe3JPi1GtLulJq7LXuM9tz_ULkox1uw_uY127rdMFfZgHYRGGRRCOBjD_CcdBOAmbk3vM8yC8CJsX95iXQXgVNq9uMufdn68AAAD__5mfNlw=
+
+# The single join reader should be on node 5, and doesn't need to output v.
+query T
+SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 40 AND v < 50 ORDER BY v]
+----
+https://cockroachdb.github.io/distsqlplan/decode.html?eJyUkT1rwzAQhvf-ivLOKrFsumjKmg5JCd2KB9U6gsHRCelcWoL_e7FVqF2I24z38byPOF3g2dHeninBvEJD4RG1QojcUEocx3Ze2rkPmEKh9aGXsV0rNBwJ5gJppSMY7PmBw6aEgiOxbTetDQrcyw-UxJ4IphrULFivB7_Yt46OZB3FTbGIx_tWoHDoxdxvNa7Z9C22J279t0wvZSG2Zxs_Z8rqqrJcKP-43JFSYJ_oX6crhlqB3Iny7yTuY0PPkZtJk8vDxE0NR0nytMrFzufR-MA5rFfhch0uV-HiF1wPd18BAAD__56h0KE=

pkg/sql/opt/exec/execbuilder/testdata/distsql_scrub

@@ -0,0 +1,53 @@
+# LogicTest: 5node-distsql-opt
+
+# TODO(radu): enable these tests when we plan merge joins.
+#
+# # Verify the index check execution plan uses a merge join.
+# 
+# statement ok
+# CREATE TABLE test (k INT PRIMARY KEY, v INT, data INT, INDEX secondary (v) STORING (data))
+# 
+# query T
+# SELECT "URL" FROM [EXPLAIN (DISTSQL)
+#     SELECT leftside.v, leftside.k, leftside.data, rightside.v, rightside.k, rightside.data
+#     FROM
+#       (SELECT v,k,data FROM test@{FORCE_INDEX=[1],NO_INDEX_JOIN} ORDER BY v,k,data) AS leftside
+#     FULL OUTER JOIN
+#       (SELECT v,k,data FROM test@{FORCE_INDEX=[2],NO_INDEX_JOIN} ORDER BY v,k,data) AS rightside
+#       ON leftside.v = rightside.v AND leftside.k = rightside.k AND leftside.data = rightside.data
+#     WHERE (leftside.k IS NULL) OR
+#           (rightside.k IS NULL)
+# ]
+# ----
+# https://cockroachdb.github.io/distsqlplan/decode.html?eJyckc2K2zAQgO99CjGnLBlIJDs9CAq6dCFLGpdscio-uNY0a3AkM5Khy5J3L45hNw5x2vQ4I33zzc8bOG9pXRwogP4BEnKEhn1JIXjuUv2Hpf0Neo5QuaaNXTpHKD0T6DeIVawJNGyLnzVtqLDEszkgWIpFVZ_KNlwdCn41kUIEhKyNWhiFRqJJID8i-DZ-FA6x2BNoecR_lz97jsQzOfQaOUWjpmiS6ahG3aM5n1ENXYFK7-zdUyb_MWUyPiXCoYjli6jJaaFGremo9UPWOs-WmOzAlnfk375caf0b8Z6efOWIZ-mw_-1rQ1o87lYrke22XzfiKVuuAaGmX3FyNtzDF672L8MUIDxWdSTWYmKUWD6L9W61ehDZRkzM4j1-P4fE7iIJmhTNAs3n0Q0t7rnLhkLjXaDLTV2tPO_WQ3ZP_bqDb7mk7-zLk6YPsxN3SlgKsX-VfbB0_VPX4Dksb8LpAJaXsLoJJ7fNyR1mdQmnN-HFhTk_fvoTAAD__3P7gDg=
+# 
+# # Verify the foreign key check execution plan uses a merge join.
+# 
+# statement ok
+# CREATE TABLE parent (
+#   id INT PRIMARY KEY,
+#   id2 INT,
+#   UNIQUE INDEX (id, id2)
+# )
+# 
+# statement ok
+# CREATE TABLE child (
+#   child_id INT PRIMARY KEY,
+#   id INT,
+#   id2 INT,
+#   FOREIGN KEY (id, id2) REFERENCES parent (id, id2)
+# )
+# 
+# query T
+# SELECT "URL" FROM [EXPLAIN (DISTSQL)
+#     SELECT p.child_id, p.id, p.id2
+#     FROM
+#       (SELECT child_id, id, id2 FROM child@{NO_INDEX_JOIN} ORDER BY id, id2) AS p
+#     FULL OUTER JOIN
+#       (SELECT id, id2 FROM parent@{FORCE_INDEX=[2],NO_INDEX_JOIN} ORDER BY id, id2) AS c
+#       ON p.id = c.id AND p.id2 = c.id2
+#     WHERE (p.id IS NOT NULL OR p.id2 IS NOT NULL) AND
+#           c.id IS NULL AND c.id2 IS NULL
+# ]
+# ----
+# https://cockroachdb.github.io/distsqlplan/decode.html?eJycklFrnTAUx9_3KcJ58nID1bi9BAYZbAWL0-G8T0PEmXNtqEskidBS_O7DCGstvRvdY345__wO5-QRtJFYdL_QAf8BCTQUJmt6dM7YFW0FmbwHHlNQepr9ihsKvbEI_BG88iMCh7r7OWKFnUR7FQMFib5TY3i2v1WjbLvZm1Zpifft-a5VsrV4bqfOovYiVECzUDCzf3I43w0IPFno__WR7PvYZKtaSdbe4YPYyEUxuyh-8s3aWIkW5c7VrMl_lbzS_Ve0A94YpdFesX339cOEnFyf8pyUp_pLRW7KrAAKI559JNiRivR4-GjVcOsjkRypYMcDULhWo0fLSRRFgpHsOynKmhSnPD-QsiKRSHfsQD4Vn0kk3gf6nHz4Q4BCOXtOREIFoyK9OL70LXur0E1GO3w5xldfjtfZoRxw24Uzs-3xmzV90GzHMuQCkOj8dsu2Q6bDVfhYz8PJG8LsZZj9NZzuwvHSLO9-BwAA__9_viDb

pkg/storage/batcheval/cmd_push_txn.go

@@ -205,8 +205,8 @@ func PushTxn(
 		if !pusherWins {
 			s = "failed to push"
 		}
-		log.Infof(ctx, "%s "+s+" %s: %s (pushee last active: %s)",
-			args.PusherTxn.Short(), args.PusheeTxn.Short(),
+		log.Infof(ctx, "%s "+s+" (push type=%s) %s: %s (pushee last active: %s)",
+			args.PusherTxn.Short(), args.PushType, args.PusheeTxn.Short(),
 			reason, reply.PusheeTxn.LastActive())
 	}