Merge #137618 #137697

137618: admission: turn on snapshot ingest bandwidth control by default r=sumeerbhola a=aadityasondhi

With #133310 merged, and promising results from internal experimentation, we can turn this setting on by default.

Informs: #121779.

Release note (ops change):
`kvadmission.store.snapshot_ingest_bandwidth_control.enabled` is now 'true' by default. This will enable disk bandwidth based Admission Control for range snapshot ingests. It requires the provisioned bandwidth to be set using `kvadmission.store.provisioned_bandwidth`.

137697: build: add configuration to opt into PGO r=rail a=rickystewart

Epic: CRDB-41952
Part of: CRDB-44693
Release note: None

Co-authored-by: Aaditya Sondhi <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

.bazelrc

@@ -119,6 +119,8 @@ build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/b
 build:macos --linkopt="-Xlinker"
 build:macos --linkopt="-no_warn_duplicate_libraries"
 
+build:pgo --@io_bazel_rules_go//go/config:pgoprofile=@pgo_profile//:profile.pprof
+
 build:engflowbase --define=EXECUTOR=remote
 build:engflowbase --disk_cache=
 build:engflowbase --experimental_inmemory_dotd_files

WORKSPACE

@@ -640,6 +640,14 @@ load("//build/bazelutil:repositories.bzl", "distdir_repositories")
 
 distdir_repositories()
 
+load("//build:pgo.bzl", "pgo_profile")
+
+pgo_profile(
+    name = "pgo_profile",
+    url = "https://storage.googleapis.com/cockroach-profiles/20241218165031.pprof",
+    sha256 = "976ceda1deda3494db56cbd95e7e2399d432f2d02c45da04d22b4e6b8eafe72d",
+)
+
 # Download and register the FIPS enabled Go toolchain at the end to avoid toolchain conflicts for gazelle.
 go_download_sdk(
     name = "go_sdk_fips",

build/pgo.bzl

@@ -0,0 +1,17 @@
+_build_bazel_content = """exports_files(["profile.pprof"])\n"""
+
+def _impl(repository_ctx):
+    repository_ctx.download(
+        url = repository_ctx.attr.url,
+        output = "profile.pprof",
+        sha256 = repository_ctx.attr.sha256,
+    )
+    repository_ctx.file("BUILD.bazel", _build_bazel_content)
+
+pgo_profile = repository_rule(
+    implementation = _impl,
+    attrs = {
+        "url": attr.string(mandatory=True),
+        "sha256": attr.string(mandatory=True),
+    },
+)

docs/generated/settings/settings.html

@@ -130,7 +130,7 @@
 <tr><td><div id="setting-kv-transaction-write-pipelining-enabled" class="anchored"><code>kv.transaction.write_pipelining.enabled<br />(alias: kv.transaction.write_pipelining_enabled)</code></div></td><td>boolean</td><td><code>true</code></td><td>if enabled, transactional writes are pipelined through Raft consensus</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-kv-transaction-write-pipelining-max-batch-size" class="anchored"><code>kv.transaction.write_pipelining.max_batch_size<br />(alias: kv.transaction.write_pipelining_max_batch_size)</code></div></td><td>integer</td><td><code>128</code></td><td>if non-zero, defines that maximum size batch that will be pipelined through Raft consensus</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-kvadmission-store-provisioned-bandwidth" class="anchored"><code>kvadmission.store.provisioned_bandwidth</code></div></td><td>byte size</td><td><code>0 B</code></td><td>if set to a non-zero value, this is used as the provisioned bandwidth (in bytes/s), for each store. It can be overridden on a per-store basis using the --store flag. Note that setting the provisioned bandwidth to a positive value may enable disk bandwidth based admission control, since admission.disk_bandwidth_tokens.elastic.enabled defaults to true</td><td>Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-kvadmission-store-snapshot-ingest-bandwidth-control-enabled" class="anchored"><code>kvadmission.store.snapshot_ingest_bandwidth_control.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set to true, snapshot ingests will be subject to disk write control in AC</td><td>Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-kvadmission-store-snapshot-ingest-bandwidth-control-enabled" class="anchored"><code>kvadmission.store.snapshot_ingest_bandwidth_control.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set to true, snapshot ingests will be subject to disk write control in AC</td><td>Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-obs-tablemetadata-automatic-updates-enabled" class="anchored"><code>obs.tablemetadata.automatic_updates.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>enables automatic updates of the table metadata cache system.table_metadata</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-obs-tablemetadata-data-valid-duration" class="anchored"><code>obs.tablemetadata.data_valid_duration</code></div></td><td>duration</td><td><code>20m0s</code></td><td>the duration for which the data in system.table_metadata is considered valid</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-schedules-backup-gc-protection-enabled" class="anchored"><code>schedules.backup.gc_protection.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>enable chaining of GC protection across backups run as part of a schedule</td><td>Serverless/Dedicated/Self-Hosted</td></tr>

pkg/util/admission/snapshot_queue.go

@@ -49,8 +49,7 @@ var snapshotWorkItemPool = sync.Pool{
 var DiskBandwidthForSnapshotIngest = settings.RegisterBoolSetting(
 	settings.SystemOnly, "kvadmission.store.snapshot_ingest_bandwidth_control.enabled",
 	"if set to true, snapshot ingests will be subject to disk write control in AC",
-	// TODO(aaditya): Enable by default once enough experimentation is done.
-	metamorphic.ConstantWithTestBool("kvadmission.store.snapshot_ingest_bandwidth_control.enabled", false),
+	metamorphic.ConstantWithTestBool("kvadmission.store.snapshot_ingest_bandwidth_control.enabled", true),
 	settings.WithPublic,
 )