Merge #61378 #65648 #66273

61378: log,debug: fix `logspy`, new HTTP API to change `vmodule` r=stevendanna,tbg,andreimatei a=miretskiy

Fixes #61335.
First 3 commits from #66096.

**tldr: the logspy endpoint does not enable maximum verbosity any more, and now returns JSON entries. Use the new vmodule endpoint to configure verbosity if needed.**

See the commit messages of the last 2 commits for details and rationales.

Relevant user-facing changes:

**Release note (security update):** All the logging output to files
or network sinks was previously disabled temporarily while an operator
was using the `/debug/logspy` HTTP API, resulting in lost entries
and a breach of auditability guarantees. This behavior has been corrected.

**Release note (bug fix):** Log entries are not lost any more while the
`/debug/logspy` HTTP API is being used. This bug had existed since
CockroachDB v1.1.

**Release note (api change):** The `/debug/logspy` HTTP API has changed.
The endpoint now returns JSON data by default.
This change is motivated as follows:

- the previous format, `crdb-v1`, cannot be parsed reliably.
- using JSON entries guarantees that the text of each entry
  all fits on a single line of output (newline characters
  inside the messages are escaped). This makes filtering
  easier and more reliable.
- using JSON enables the user to apply `jq` on the output, for
  example via `curl -s .../debug/logspy | jq ...`

If the previous format is desired, the user can pass the query
argument `&flatten=1` to the `logspy` URL to obtain the previous flat
text format (`crdb-v1`) instead.

**Release note (api change)**: The `/debug/logspy` API does not any more
enable maximum logging verbosity automatically. To change the
verbosity, use the new `/debug/vmodule` endpoint or pass the
`&vmodule=` query parameter to the `/debug/logspy` endpoint.

For example, suppose you wish to run a 20s logspy session:

- Before:

  ```
  curl 'https://.../debug/logspy?duration=20s&...'
  ```

- Now:

  ```
  curl 'https://.../debug/logspy?duration=20s&vmodule=...'
  ```

  OR

  ```
  curl 'https://.../debug/vmodule?duration=22s&vmodule=...'
  curl 'https://.../debug/logspy?duration=20s'
  ```

As for the regular `vmodule` command-line flag, the maximum verbosity
across all the source code can be selected with the pattern `*=4`.

Note: at most one in-flight HTTP API request is allowed to modify the
`vmodule` parameter. This maintain the invariant that the
configuration restored at the end of each request is the same as when
the request started.

**Release note (api change)**: The new `/debug/vmodule` API makes it
possible for an operator to configure the logging verbosity in a
similar way as the SQL built-in function
`crdb_internal.set_vmodule()`, or to query the current configuration
as in `crdb_internal.get_vmodule()`. Additionally, any configuration
change performed via this API can be automatically reverted after a
configurable delay. The API forms are:

- `/debug/vmodule` - retrieve the current configuration.
- `/debug/vmodule?set=[vmodule config]&duration=[duration]` - change
  the configuration to `[vmodule config]` . The previous configuration
  at the time the `/debug/vmodule` request started is restored after
  `[duration]`. This duration, if not specified, defaults to twice the
  default duration of a `logspy` request (currently, the `logspy`
  default duration is 5s, so the `vmodule` default duration is 10s).
  If the duration is zero or negative, the previous configuration
  is never restored.

65648: sqlliveness/slstorage: break InternalExecutor depedency r=knz a=ajwerner

There was no fundamental reason to rely on SQL for this low-level subsystem.

Release note: None

66273: storageccl: don't update file registry when creating unencrypted files r=jbowens a=andyyang890

This patch changes PebbleFileRegistry to not store an entry
for an unencrypted file. This should lead to a performance
improvement since the file registry is rewritten every time
it is updated.

Fixes #65430.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Andy Yang <[email protected]>

pkg/ccl/storageccl/engineccl/encrypted_fs_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
+	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/pebble"
@@ -280,3 +281,101 @@ func TestPebbleEncryption(t *testing.T) {
 
 	db.Close()
 }
+
+func TestPebbleEncryption2(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	memFS := vfs.NewMem()
+	firstKeyFile128 := "111111111111111111111111111111111234567890123456"
+	secondKeyFile128 := "111111111111111111111111111111198765432198765432"
+	writeToFile(t, memFS, "16v1.key", []byte(firstKeyFile128))
+	writeToFile(t, memFS, "16v2.key", []byte(secondKeyFile128))
+
+	keys := make(map[string]bool)
+	validateKeys := func(reader storage.Reader) bool {
+		keysCopy := make(map[string]bool)
+		for k, v := range keys {
+			keysCopy[k] = v
+		}
+
+		foundUnknown := false
+		kvFunc := func(kv roachpb.KeyValue) error {
+			key := kv.Key
+			val := kv.Value
+			expected := keysCopy[string(key)]
+			if !expected || len(val.RawBytes) == 0 {
+				foundUnknown = true
+				return nil
+			}
+			delete(keysCopy, string(key))
+			return nil
+		}
+
+		_, err := storage.MVCCIterate(
+			context.Background(),
+			reader,
+			nil,
+			roachpb.KeyMax,
+			hlc.Timestamp{},
+			storage.MVCCScanOptions{},
+			kvFunc,
+		)
+		require.NoError(t, err)
+		return len(keysCopy) == 0 && !foundUnknown
+	}
+
+	addKeyAndValidate := func(
+		key string, val string, encKeyFile string, oldEncFileKey string,
+	) {
+		encOptions := baseccl.EncryptionOptions{
+			KeySource: baseccl.EncryptionKeySource_KeyFiles,
+			KeyFiles: &baseccl.EncryptionKeyFiles{
+				CurrentKey: encKeyFile,
+				OldKey:     oldEncFileKey,
+			},
+			DataKeyRotationPeriod: 1000,
+		}
+		encOptionsBytes, err := protoutil.Marshal(&encOptions)
+		require.NoError(t, err)
+
+		opts := storage.DefaultPebbleOptions()
+		opts.FS = memFS
+		opts.Cache = pebble.NewCache(1 << 20)
+		defer opts.Cache.Unref()
+
+		db, err := storage.NewPebble(
+			context.Background(),
+			storage.PebbleConfig{
+				StorageConfig: base.StorageConfig{
+					Attrs:             roachpb.Attributes{},
+					MaxSize:           512 << 20,
+					UseFileRegistry:   true,
+					EncryptionOptions: encOptionsBytes,
+				},
+				Opts: opts,
+			})
+		require.NoError(t, err)
+		require.True(t, validateKeys(db))
+
+		keys[key] = true
+		err = storage.MVCCPut(
+			context.Background(),
+			db,
+			nil, /* ms */
+			roachpb.Key(key),
+			hlc.Timestamp{},
+			roachpb.MakeValueFromBytes([]byte(val)),
+			nil, /* txn */
+		)
+		require.NoError(t, err)
+		require.NoError(t, db.Flush())
+		require.NoError(t, db.Compact())
+		require.True(t, validateKeys(db))
+		db.Close()
+	}
+
+	addKeyAndValidate("a", "a", "16v1.key", "plain")
+	addKeyAndValidate("b", "b", "plain", "16v1.key")
+	addKeyAndValidate("c", "c", "16v2.key", "plain")
+	addKeyAndValidate("d", "d", "plain", "16v2.key")
+}

pkg/jobs/registry_external_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
+	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -111,7 +112,7 @@ func TestRegistryResumeExpiredLease(t *testing.T) {
 		idContainer := base.NewSQLIDContainer(0, &c)
 		ac := log.AmbientContext{Tracer: tracing.NewTracer()}
 		sqlStorage := slstorage.NewStorage(
-			s.Stopper(), clock, db, s.InternalExecutor().(sqlutil.InternalExecutor), s.ClusterSettings(),
+			s.Stopper(), clock, db, keys.SystemSQLCodec, s.ClusterSettings(),
 		)
 		sqlInstance := slinstance.NewSQLInstance(s.Stopper(), clock, sqlStorage, s.ClusterSettings())
 		r := jobs.MakeRegistry(

pkg/jobs/registry_test.go

@@ -71,7 +71,7 @@ func TestRegistryCancelation(t *testing.T) {
 		roachpb.Version{Major: 20, Minor: 1},
 		roachpb.Version{Major: 20, Minor: 1},
 		true)
-	sqlStorage := slstorage.NewStorage(stopper, clock, db, nil, settings)
+	sqlStorage := slstorage.NewStorage(stopper, clock, db, keys.SystemSQLCodec, settings)
 	sqlInstance := slinstance.NewSQLInstance(stopper, clock, sqlStorage, settings)
 	registry := MakeRegistry(
 		log.AmbientContext{},

pkg/server/debug/BUILD.bazel

@@ -6,6 +6,7 @@ go_library(
         "cpuprofile.go",
         "logspy.go",
         "server.go",
+        "vmodule.go",
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/server/debug",
     visibility = ["//visibility:public"],
@@ -26,6 +27,7 @@ go_library(
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_pebble//tool",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_rcrowley_go_metrics//:go-metrics",
         "@com_github_rcrowley_go_metrics//exp",
         "@com_github_spf13_cobra//:cobra",