From ee5cce5dd2abc382baf67c671223ddcbbe8a9bf2 Mon Sep 17 00:00:00 2001
From: Bilal Akhtar <bilal@cockroachlabs.com>
Date: Wed, 16 Aug 2023 15:53:38 -0400
Subject: [PATCH] storage: Write sstables in v4 format for shared ingestions

We require sstables to be written with obsolete bits
for shared sstable ingestion to work correctly. This
change moves to writing sstables with the new format
if the cluster version is high enough.

Unblocks #107394.

Epic: none

Release note: None
---
 pkg/kv/kvnemesis/testdata/TestApplier/addsstable  |  2 +-
 .../testdata/TestValidate/addsstable_ingestion    |  2 +-
 .../addsstable_ingestion_shadowing_scan           |  2 +-
 pkg/storage/sst_writer.go                         |  3 +++
 pkg/storage/sst_writer_test.go                    | 15 ++++++++++++++-
 5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/pkg/kv/kvnemesis/testdata/TestApplier/addsstable b/pkg/kv/kvnemesis/testdata/TestApplier/addsstable
index 62460a045987..fa2b430fd0bd 100644
--- a/pkg/kv/kvnemesis/testdata/TestApplier/addsstable
+++ b/pkg/kv/kvnemesis/testdata/TestApplier/addsstable
@@ -1,6 +1,6 @@
 echo
 ----
-db1.AddSSTable(ctx, tk(1), tk(4), ... /* @s1 */) // 1115 bytes (as writes)
+db1.AddSSTable(ctx, tk(1), tk(4), ... /* @s1 */) // 1144 bytes (as writes)
 // ^-- tk(1) -> sv(s1): /Table/100/"0000000000000001"/<ts> -> /BYTES/v1
 // ^-- tk(2) -> sv(s1): /Table/100/"0000000000000002"/<ts> -> /<empty>
 // ^-- [tk(3), tk(4)) -> sv(s1): /Table/100/"000000000000000{3"-4"} -> /<empty>
diff --git a/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion b/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion
index b5db36ec4ae8..a5bee971532d 100644
--- a/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion
+++ b/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion
@@ -1,6 +1,6 @@
 echo
 ----
-db0.AddSSTable(ctx, tk(1), tk(4), ... /* @s1 */) // 1115 bytes
+db0.AddSSTable(ctx, tk(1), tk(4), ... /* @s1 */) // 1144 bytes
 // ^-- tk(1) -> sv(s1): /Table/100/"0000000000000001"/0.000000001,0 -> /BYTES/v1
 // ^-- tk(2) -> sv(s1): /Table/100/"0000000000000002"/0.000000001,0 -> /<empty>
 // ^-- [tk(3), tk(4)) -> sv(s1): /Table/100/"000000000000000{3"-4"} -> /<empty>
diff --git a/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion_shadowing_scan b/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion_shadowing_scan
index 186c7577c771..a18351af1db6 100644
--- a/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion_shadowing_scan
+++ b/pkg/kv/kvnemesis/testdata/TestValidate/addsstable_ingestion_shadowing_scan
@@ -4,7 +4,7 @@ db0.Put(ctx, tk(1), sv(1)) // @0.000000001,0 <nil>
 db0.Put(ctx, tk(2), sv(1)) // @0.000000001,0 <nil>
 db0.Put(ctx, tk(3), sv(1)) // @0.000000001,0 <nil>
 db0.Put(ctx, tk(4), sv(1)) // @0.000000001,0 <nil>
-db0.AddSSTable(ctx, tk(1), tk(4), ... /* @s2 */) // 1116 bytes
+db0.AddSSTable(ctx, tk(1), tk(4), ... /* @s2 */) // 1145 bytes
 // ^-- tk(1) -> sv(s2): /Table/100/"0000000000000001"/0.000000001,0 -> /BYTES/v2
 // ^-- tk(2) -> sv(s2): /Table/100/"0000000000000002"/0.000000001,0 -> /<empty>
 // ^-- [tk(3), tk(4)) -> sv(s2): /Table/100/"000000000000000{3"-4"} -> /<empty>
diff --git a/pkg/storage/sst_writer.go b/pkg/storage/sst_writer.go
index 01538caeafa5..3d0c08a8de12 100644
--- a/pkg/storage/sst_writer.go
+++ b/pkg/storage/sst_writer.go
@@ -74,6 +74,9 @@ 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) {
+		format = sstable.TableFormatPebblev4
+	}
 	opts := DefaultPebbleOptions().MakeWriterOptions(0, format)
 	opts.MergerName = "nullptr"
 	return opts
diff --git a/pkg/storage/sst_writer_test.go b/pkg/storage/sst_writer_test.go
index 0dd72bcf672e..e64e02680fe3 100644
--- a/pkg/storage/sst_writer_test.go
+++ b/pkg/storage/sst_writer_test.go
@@ -95,12 +95,25 @@ func TestMakeIngestionWriterOptions(t *testing.T) {
 		{
 			name: "with value blocks",
 			st: func() *cluster.Settings {
-				st := cluster.MakeTestingClusterSettings()
+				st := cluster.MakeTestingClusterSettingsWithVersions(
+					clusterversion.ByKey(clusterversion.V23_1EnablePebbleFormatSSTableValueBlocks),
+					clusterversion.TestingBinaryMinSupportedVersion,
+					true,
+				)
 				ValueBlocksEnabled.Override(context.Background(), &st.SV, true)
 				return st
 			}(),
 			want: sstable.TableFormatPebblev3,
 		},
+		{
+			name: "with virtual sstables",
+			st: func() *cluster.Settings {
+				st := cluster.MakeTestingClusterSettings()
+				ValueBlocksEnabled.Override(context.Background(), &st.SV, true)
+				return st
+			}(),
+			want: sstable.TableFormatPebblev4,
+		},
 	}
 
 	for _, tc := range testCases {