storage: misc fixes after excise cluster setting addition

docs/generated/settings/settings-for-tenants.txt

@@ -320,4 +320,4 @@ trace.snapshot.rate	duration	0s	if non-zero, interval at which background trace
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez	application
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.	application
 ui.display_timezone	enumeration	etc/utc	the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]	application
-version	version	1000023.1-26	set the active cluster version in the format '<major>.<minor>'	application
+version	version	1000023.1-28	set the active cluster version in the format '<major>.<minor>'	application

docs/generated/settings/settings.html

@@ -256,7 +256,7 @@
 <tr><td><div id="setting-sql-ttl-job-enabled" class="anchored"><code>sql.ttl.job.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>whether the TTL job is enabled</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-txn-read-committed-syntax-enabled" class="anchored"><code>sql.txn.read_committed_syntax.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commands</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-txn-fingerprint-id-cache-capacity" class="anchored"><code>sql.txn_fingerprint_id_cache.capacity</code></div></td><td>integer</td><td><code>100</code></td><td>the maximum number of txn fingerprint IDs stored</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-storage-experimental-eventually-file-only-snapshots-enabled" class="anchored"><code>storage.experimental.eventually_file_only_snapshots.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to use eventually-file-only-snapshots</td><td>Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-storage-experimental-eventually-file-only-snapshots-enabled" class="anchored"><code>storage.experimental.eventually_file_only_snapshots.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to use eventually-file-only-snapshots even when kv.snapshot_receiver.excise.enabled is false</td><td>Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-storage-max-sync-duration" class="anchored"><code>storage.max_sync_duration</code></div></td><td>duration</td><td><code>20s</code></td><td>maximum duration for disk operations; any operations that take longer than this setting trigger a warning log entry or process crash</td><td>Serverless/Dedicated/Self-Hosted (read-only)</td></tr>
 <tr><td><div id="setting-storage-max-sync-duration-fatal-enabled" class="anchored"><code>storage.max_sync_duration.fatal.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if true, fatal the process when a disk operation exceeds storage.max_sync_duration</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-storage-value-blocks-enabled" class="anchored"><code>storage.value_blocks.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>set to true to enable writing of value blocks in sstables</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
@@ -269,6 +269,6 @@
 <tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-display-timezone" class="anchored"><code>ui.display_timezone</code></div></td><td>enumeration</td><td><code>etc/utc</code></td><td>the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-26</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-28</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -499,6 +499,12 @@ const (
 	// role for all existing functions.
 	V23_2_GrantExecuteToPublic
 
+	// V23_2_EnablePebbleFormatVirtualSSTables enables the Pebble
+	// FormatMajorVersion for virtual sstables. Note that the ratcheting for the
+	// format major version in Pebble should have happened with
+	// V23_2_PebbleFormatVirtualSSTables above.
+	V23_2_EnablePebbleFormatVirtualSSTables
+
 	// *************************************************
 	// Step (1) Add new versions here.
 	// Do not add new versions to a patch release.
@@ -832,6 +838,10 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     V23_2_GrantExecuteToPublic,
 		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 26},
 	},
+	{
+		Key:     V23_2_EnablePebbleFormatVirtualSSTables,
+		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 28},
+	},
 
 	// *************************************************
 	// Step (2): Add new versions here.

pkg/kv/kvserver/store_snapshot.go

@@ -505,7 +505,7 @@ func (kvSS *kvBatchSnapshotStrategy) Receive(
 	keyRanges := rditer.MakeReplicatedKeySpans(header.State.Desc)
 
 	doExcise := header.SharedReplicate || (storage.UseExciseForSnapshots.Get(&s.ClusterSettings().SV) &&
-		s.cfg.Settings.Version.IsActive(ctx, clusterversion.V23_2_PebbleFormatVirtualSSTables))
+		s.cfg.Settings.Version.IsActive(ctx, clusterversion.V23_2_EnablePebbleFormatVirtualSSTables))
 	if header.SharedReplicate && !s.cfg.SharedStorageEnabled {
 		return noSnap, sendSnapshotError(ctx, s, stream, errors.New("cannot accept shared sstables"))
 	}

pkg/storage/pebble.go

@@ -108,7 +108,7 @@ var ValueBlocksEnabled = settings.RegisterBoolSetting(
 var UseEFOS = settings.RegisterBoolSetting(
 	settings.SystemOnly,
 	"storage.experimental.eventually_file_only_snapshots.enabled",
-	"set to true to use eventually-file-only-snapshots",
+	"set to true to use eventually-file-only-snapshots even when kv.snapshot_receiver.excise.enabled is false",
 	util.ConstantWithMetamorphicTestBool(
 		"storage.experimental.eventually_file_only_snapshots.enabled", false), /* defaultValue */
 	settings.WithPublic)

pkg/storage/sst_writer.go

@@ -73,7 +73,7 @@ func MakeIngestionWriterOptions(ctx context.Context, cs *cluster.Settings) sstab
 		ValueBlocksEnabled.Get(&cs.SV) {
 		format = sstable.TableFormatPebblev3
 	}
-	if cs.Version.IsActive(ctx, clusterversion.V23_2_PebbleFormatVirtualSSTables) && ValueBlocksEnabled.Get(&cs.SV) {
+	if cs.Version.IsActive(ctx, clusterversion.V23_2_EnablePebbleFormatVirtualSSTables) && ValueBlocksEnabled.Get(&cs.SV) {
 		format = sstable.TableFormatPebblev4
 	}
 	opts := DefaultPebbleOptions().MakeWriterOptions(0, format)