Merge #58217 #58247

58217: sql/logictest: add a directive to skip the rest of a logictest on retry r=ajwerner a=ajwerner

Some logictests, specifically schema_change_in_txn, periodically experience
completely valid serializable restarts. We don't have a way to allow the
test to proceed and retry in those cases. Instead, this allows the test to
indicate that this happening is fine and to report instead that the test has
been skipped.

Touches #53724.

Release note: None

58247: vendor: Bump pebble to f614b5ad0faa88f794548581b80ad05f99e044ba r=itsbilal a=itsbilal

Changes pulled in:
```
f614b5ad0faa88f794548581b80ad05f99e044ba iterator: use i.pos in sampling
23ad85a4e5a6da313d31c0601f6b890d331c529b options: enable read based compactions
4c4e0431d28cc074ca949df757bc8c4b4aba1b72 db: return error if manual compaction start >= end
6f499ee95cfe97aa254a5f48d5dffd106662956b sstable: use fixed error in Close
```

Release note (performance improvement): Allow the storage engine
to compact sstables based on reads, on read-heavy workloads.

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>

go.mod

@@ -40,7 +40,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.13.0
 	github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f
-	github.com/cockroachdb/pebble v0.0.0-20201221143129-4b1164f6ed2d
+	github.com/cockroachdb/pebble v0.0.0-20201223160501-f614b5ad0faa
 	github.com/cockroachdb/redact v1.0.9
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2
@@ -153,7 +153,7 @@ require (
 	golang.org/x/oauth2 v0.0.0-20190115181402-5dab4167f31c
 	golang.org/x/perf v0.0.0-20180704124530-6e6d33e29852
 	golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9
-	golang.org/x/sys v0.0.0-20201221093633-bc327ba9c2f0
+	golang.org/x/sys v0.0.0-20201223074533-0d417f636930
 	golang.org/x/text v0.3.3
 	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
 	golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347

go.sum

@@ -161,8 +161,8 @@ github.com/cockroachdb/grpc-gateway v1.14.6-0.20200519165156-52697fc4a249 h1:pZu
 github.com/cockroachdb/grpc-gateway v1.14.6-0.20200519165156-52697fc4a249/go.mod h1:UJ0EZAp832vCd54Wev9N1BMKEyvcZ5+IM0AwDrnlkEc=
 github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f h1:o/kfcElHqOiXqcou5a3rIlMc7oJbMQkeLk0VQJ7zgqY=
 github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
-github.com/cockroachdb/pebble v0.0.0-20201221143129-4b1164f6ed2d h1:ivMk4boQafPxIm0JZVRDd7+Gfu7r8C9r3yDlmgSRr3I=
-github.com/cockroachdb/pebble v0.0.0-20201221143129-4b1164f6ed2d/go.mod h1:c3G8ud5zF3+nYHCWmVmtsA8eEtjrDSa6qeLtcRZyevE=
+github.com/cockroachdb/pebble v0.0.0-20201223160501-f614b5ad0faa h1:ucQr+rdfI+sH8fNMAon3u0xnMkuxSoF2x67l/HhsnZw=
+github.com/cockroachdb/pebble v0.0.0-20201223160501-f614b5ad0faa/go.mod h1:c3G8ud5zF3+nYHCWmVmtsA8eEtjrDSa6qeLtcRZyevE=
 github.com/cockroachdb/redact v1.0.8 h1:8QG/764wK+vmEYoOlfobpe12EQcS81ukx/a4hdVMxNw=
 github.com/cockroachdb/redact v1.0.8/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.0.9 h1:sjlUvGorKMIVQfo+w2RqDi5eewCHn453C/vdIXMzjzI=
@@ -851,8 +851,8 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201009025420-dfb3f7c4e634/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20201221093633-bc327ba9c2f0 h1:n+DPcgTwkgWzIFpLmoimYR2K2b0Ga5+Os4kayIN0vGo=
-golang.org/x/sys v0.0.0-20201221093633-bc327ba9c2f0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20201223074533-0d417f636930 h1:vRgIt+nup/B/BwIS0g2oC0haq0iqbV3ZA+u6+0TlNCo=
+golang.org/x/sys v0.0.0-20201223074533-0d417f636930/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=

pkg/sql/logictest/logic.go

@@ -1111,6 +1111,12 @@ type logicTest struct {
 
 	curPath   string
 	curLineNo int
+
+	// skipOnRetry is explicitly set to true by the skip_on_retry directive.
+	// If true, serializability violations in the test when unexpected cause the
+	// entire test to be skipped and the below skippedOnRetry to be set to true.
+	skipOnRetry    bool
+	skippedOnRetry bool
 }
 
 func (t *logicTest) t() *testing.T {
@@ -1712,6 +1718,7 @@ func (t *logicTest) processTestFile(path string, config testClusterConfig) error
 					t.Error(err)
 				}
 			})
+			t.maybeSkipOnRetry(nil)
 		}
 	}
 
@@ -1829,6 +1836,8 @@ func (t *logicTest) processSubtest(
 				)
 			}
 			repeat = count
+		case "skip_on_retry":
+			t.skipOnRetry = true
 
 		case "sleep":
 			var err error
@@ -2286,6 +2295,17 @@ func (t *logicTest) processSubtest(
 	return s.Err()
 }
 
+// Some tests encounter serializability failures sometimes and indicate
+// that they want to just get skipped when that happens.
+func (t *logicTest) maybeSkipOnRetry(err error) {
+	if pqErr := (*pq.Error)(nil); t.skippedOnRetry ||
+		(t.skipOnRetry && errors.As(err, &pqErr) &&
+			pgcode.MakeCode(string(pqErr.Code)) == pgcode.SerializationFailure) {
+		t.skippedOnRetry = true
+		skip.WithIssue(t.t(), 53724)
+	}
+}
+
 // verifyError checks that either:
 // - no error was found when none was expected, or
 // - in case no error was found, a notice was found when one was expected, or
@@ -2298,6 +2318,7 @@ func (t *logicTest) verifyError(
 	sql, pos, expectNotice, expectErr, expectErrCode string, err error,
 ) (bool, error) {
 	if expectErr == "" && expectErrCode == "" && err != nil {
+		t.maybeSkipOnRetry(err)
 		cont := t.unexpectedError(sql, pos, err)
 		if cont {
 			// unexpectedError() already reported via t.Errorf. no need for more.

pkg/sql/logictest/testdata/logic_test/schema_change_in_txn

@@ -3,6 +3,11 @@
 statement ok
 SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
 
+
+# Skip the rest of the test if a retry occurs. They can happen and are fine
+# but there's no way to encapsulate that in logictests.
+skip_on_retry
+
 subtest create_and_add_fk_in_same_txn
 
 statement ok

pkg/sql/logictest/testdata/logic_test/skip_on_retry

@@ -0,0 +1,24 @@
+# LogicTest: local
+
+# This test demonstrates that the skip_on_retry directive works as expected.
+# The directive applies to the entire remainder of the file.
+
+statement ok
+BEGIN; SELECT * FROM system.namespace LIMIT 1;
+
+statement error pgcode 40001 TransactionRetryWithProtoRefreshError
+SELECT crdb_internal.force_retry('1h');
+
+statement ok
+ROLLBACK;
+
+skip_on_retry
+
+subtest skip_this_subtest
+
+statement ok
+BEGIN; SELECT * FROM system.namespace LIMIT 1;
+
+statement ok
+SELECT crdb_internal.force_retry('1h');
+

pkg/storage/disk_map_test.go

@@ -235,7 +235,10 @@ func TestPebbleMapClose(t *testing.T) {
 
 	// Force it to a lower-level. This is done so as to avoid the automatic
 	// compactions out of L0 that would normally occur.
-	if err := e.db.Compact(diskMap.makeKey([]byte{'a'}), diskMap.makeKey([]byte{'z'})); err != nil {
+	startKey := diskMap.makeKey([]byte{'a'})
+	startKeyCopy := make([]byte, len(startKey))
+	copy(startKeyCopy, startKey)
+	if err := e.db.Compact(startKeyCopy, diskMap.makeKey([]byte{'z'})); err != nil {
 		t.Fatal(err)
 	}