Merge #56064 #56092 #56095

56064: storage: Bring back MaxSyncDuration env var, fix disk-stalled roachtest r=itsbilal a=itsbilal

The disk-stalled roachtest relies on the ability to control disk stall
detection / fatal intervals, as charybdefs only injects 50ms of
delay per syscall. This change adds an env variable, similar to the
one removed in #55186 to set max sync duration, except now it governs
the default of the cluster setting. The roachtest now modifies that
env variable to let disk stall detection trip on short syscall
delays.

Fixes #54332.

Release note: None.

56092: parser: pin pkg/security as a bazel dep for pkg/parser r=irfansharif a=irfansharif

We picked up a dependency on pkg/security within the parser package
after #55398.

We have to pin go dependencies by hand if they're only present in
auto-generated code. It's because it's not otherwise visible to
bazel/gazelle when generating the BUILD files (during the analysis
phase).

Release note: None

56095: partialidx: add benchmarks for two-variable comparisons r=RaduBerinde a=mgartner

Two-variable comparison implication performs similarly to other types of
implications.

    BenchmarkImplicator/single-exact-match-16                         76.5 ns/op
    BenchmarkImplicator/single-inexact-match-16                      342 ns/op
    BenchmarkImplicator/range-inexact-match-16                       782 ns/op
    BenchmarkImplicator/two-var-comparison-16                        302 ns/op
    BenchmarkImplicator/single-exact-match-extra-filters-16          310 ns/op
    BenchmarkImplicator/single-inexact-match-extra-filters-16        609 ns/op
    BenchmarkImplicator/multi-column-and-exact-match-16               82.4 ns/op
    BenchmarkImplicator/multi-column-and-inexact-match-16            722 ns/op
    BenchmarkImplicator/multi-column-and-two-var-comparisons-16      611 ns/op
    BenchmarkImplicator/multi-column-or-exact-match-16                76.1 ns/op
    BenchmarkImplicator/multi-column-or-exact-match-reverse-16       595 ns/op
    BenchmarkImplicator/multi-column-or-inexact-match-16            1081 ns/op
    BenchmarkImplicator/in-implies-or-16                             976 ns/op
    BenchmarkImplicator/and-filters-do-not-imply-pred-16            3710 ns/op
    BenchmarkImplicator/or-filters-do-not-imply-pred-16              917 ns/op
    BenchmarkImplicator/many-columns-exact-match10-16                296 ns/op
    BenchmarkImplicator/many-columns-inexact-match10-16             6853 ns/op
    BenchmarkImplicator/many-columns-exact-match100-16             19817 ns/op
    BenchmarkImplicator/many-columns-inexact-match100-16          447894 ns/op

Release note: None

Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

pkg/cmd/roachtest/disk_stall.go

@@ -103,8 +103,7 @@ func runDiskStalledDetection(
 	go func() {
 		t.WorkerStatus("running server")
 		out, err := c.RunWithBuffer(ctx, l, n,
-			fmt.Sprintf("timeout --signal 9 %ds env COCKROACH_ENGINE_MAX_SYNC_DURATION_FATAL=true "+
-				"COCKROACH_ENGINE_MAX_SYNC_DURATION=%s COCKROACH_LOG_MAX_SYNC_DURATION=%s "+
+			fmt.Sprintf("timeout --signal 9 %ds env COCKROACH_ENGINE_MAX_SYNC_DURATION_DEFAULT=%s COCKROACH_LOG_MAX_SYNC_DURATION=%s "+
 				"./cockroach start-single-node --insecure --logtostderr=INFO --store {store-dir}/%s --log-dir {store-dir}/%s",
 				int(dur.Seconds()), maxDataSync, maxLogSync, dataDir, logDir,
 			),

pkg/sql/opt/partialidx/implicator_test.go

@@ -154,6 +154,12 @@ func BenchmarkImplicator(b *testing.B) {
 			filters: "a >= 10 AND a <= 90",
 			pred:    "a > 0 AND a < 100",
 		},
+		{
+			name:    "two-var-comparison",
+			vars:    "a int, b int",
+			filters: "a > b",
+			pred:    "b <= a",
+		},
 		{
 			name:    "single-exact-match-extra-filters",
 			vars:    "a int, b int, c int, d int, e int",
@@ -178,6 +184,12 @@ func BenchmarkImplicator(b *testing.B) {
 			filters: "a >= 10 AND b = 'foo'",
 			pred:    "a >= 0 AND b IN ('foo', 'bar')",
 		},
+		{
+			name:    "multi-column-and-two-var-comparisons",
+			vars:    "a int, b int, c int, d int",
+			filters: "a > b AND c < d",
+			pred:    "b <= a AND c != d",
+		},
 		{
 			name:    "multi-column-or-exact-match",
 			vars:    "a int, b string",

pkg/sql/parser/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "//pkg/docs",
         "//pkg/geo/geopb",  # keep
         "//pkg/roachpb",  # keep
+        "//pkg/security",  # keep
         "//pkg/sql/lex",
         "//pkg/sql/lexbase:lex",
         "//pkg/sql/pgwire/pgcode",

pkg/storage/pebble.go

@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/storage/fs"
+	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -41,10 +42,12 @@ import (
 )
 
 const (
-	maxSyncDurationDefault                = 60 * time.Second
 	maxSyncDurationFatalOnExceededDefault = true
 )
 
+// Default for MaxSyncDuration below.
+var maxSyncDurationDefault = envutil.EnvOrDefaultDuration("COCKROACH_ENGINE_MAX_SYNC_DURATION_DEFAULT", 60*time.Second)
+
 // MaxSyncDuration is the threshold above which an observed engine sync duration
 // triggers either a warning or a fatal error.
 var MaxSyncDuration = settings.RegisterDurationSetting(
@@ -358,6 +361,24 @@ func DefaultPebbleOptions() *pebble.Options {
 	// of the benefit of having bloom filters on every level for only 10% of the
 	// memory cost.
 	opts.Levels[6].FilterPolicy = nil
+
+	// Set disk health check interval to min(5s, maxSyncDurationDefault). This
+	// is mostly to ease testing; the default of 5s is too infrequent to test
+	// conveniently. See the disk-stalled roachtest for an example of how this
+	// is used.
+	diskHealthCheckInterval := 5 * time.Second
+	if diskHealthCheckInterval.Seconds() > maxSyncDurationDefault.Seconds() {
+		diskHealthCheckInterval = maxSyncDurationDefault
+	}
+	// Instantiate a file system with disk health checking enabled. This FS wraps
+	// vfs.Default, and can be wrapped for encryption-at-rest.
+	opts.FS = vfs.WithDiskHealthChecks(vfs.Default, diskHealthCheckInterval,
+		func(name string, duration time.Duration) {
+			opts.EventListener.DiskSlow(pebble.DiskSlowInfo{
+				Path:     name,
+				Duration: duration,
+			})
+		})
 	return opts
 }