sstable: remove CommonProperties

Fold back `sstable.CommonProperties` into `sstable.Properties`; we no
longer need this (somewhat artificial) separation.

Author: RaduBerinde

compaction_picker_test.go

@@ -160,9 +160,7 @@ func parseTableMeta(t *testing.T, s string, opts *Options) (*manifest.TableMetad
 				return nil, err
 			}
 			m.TableBacking.PopulateProperties(&sstable.Properties{
-				CommonProperties: sstable.CommonProperties{
-					NumDeletions: 1, // At least one range del responsible for the deletion bytes.
-				},
+				NumDeletions: 1, // At least one range del responsible for the deletion bytes.
 			})
 			m.PopulateStats(&manifest.TableStats{
 				RangeDeletionsBytesEstimate: uint64(v),

compaction_test.go

@@ -3424,12 +3424,10 @@ func TestTombstoneDensityCompactionMoveOptimization(t *testing.T) {
 	}
 	meta.InitPhysicalBacking()
 	meta.TableBacking.PopulateProperties(&sstable.Properties{
-		CommonProperties: sstable.CommonProperties{
-			NumEntries:              10,
-			NumDeletions:            8,
-			NumDataBlocks:           100,
-			NumTombstoneDenseBlocks: 90, // Above threshold
-		},
+		NumEntries:              10,
+		NumDeletions:            8,
+		NumDataBlocks:           100,
+		NumTombstoneDenseBlocks: 90, // Above threshold
 	})
 	meta.PopulateStats(&manifest.TableStats{})
 	meta.ExtendPointKeyBounds(opts.Comparer.Compare,
@@ -3526,13 +3524,11 @@ func TestTombstoneDensityCompactionMoveOptimization_NoMoveWithOverlap(t *testing
 	}
 	metaL4.InitPhysicalBacking()
 	metaL4.TableBacking.PopulateProperties(&sstable.Properties{
-		CommonProperties: sstable.CommonProperties{
-			NumEntries:   10,
-			NumDeletions: 8,
+		NumEntries:   10,
+		NumDeletions: 8,
 
-			NumDataBlocks:           100,
-			NumTombstoneDenseBlocks: 90, // Above threshold
-		},
+		NumDataBlocks:           100,
+		NumTombstoneDenseBlocks: 90, // Above threshold
 	})
 	metaL4.PopulateStats(&manifest.TableStats{})
 	metaL4.ExtendPointKeyBounds(opts.Comparer.Compare,
@@ -3610,13 +3606,11 @@ func TestTombstoneDensityCompactionMoveOptimization_GrandparentOverlapTooLarge(t
 	}
 	metaL4.InitPhysicalBacking()
 	metaL4.TableBacking.PopulateProperties(&sstable.Properties{
-		CommonProperties: sstable.CommonProperties{
-			NumEntries:   10,
-			NumDeletions: 8,
+		NumEntries:   10,
+		NumDeletions: 8,
 
-			NumDataBlocks:           100,
-			NumTombstoneDenseBlocks: 90,
-		},
+		NumDataBlocks:           100,
+		NumTombstoneDenseBlocks: 90,
 	})
 	metaL4.PopulateStats(&manifest.TableStats{})
 	metaL4.ExtendPointKeyBounds(opts.Comparer.Compare,
@@ -3677,13 +3671,11 @@ func TestTombstoneDensityCompactionMoveOptimization_BelowDensityThreshold(t *tes
 	}
 	meta.InitPhysicalBacking()
 	meta.TableBacking.PopulateProperties(&sstable.Properties{
-		CommonProperties: sstable.CommonProperties{
-			NumEntries:   10,
-			NumDeletions: 5,
+		NumEntries:   10,
+		NumDeletions: 5,
 
-			NumDataBlocks:           100,
-			NumTombstoneDenseBlocks: 50, // Below threshold
-		},
+		NumDataBlocks:           100,
+		NumTombstoneDenseBlocks: 50, // Below threshold
 	})
 	meta.PopulateStats(&manifest.TableStats{})
 	meta.ExtendPointKeyBounds(opts.Comparer.Compare,

ingest_test.go

@@ -234,11 +234,9 @@ func TestIngestLoadRand(t *testing.T) {
 			expected[i].Size = meta.Size
 			expected[i].InitPhysicalBacking()
 			expected[i].TableBacking.PopulateProperties(&sstable.Properties{
-				CommonProperties: sstable.CommonProperties{
-					RawKeySize:    rawKeySize,
-					NumEntries:    count,
-					NumDataBlocks: 1,
-				},
+				RawKeySize:    rawKeySize,
+				NumEntries:    count,
+				NumDataBlocks: 1,
 			})
 		}()
 	}

sstable/internal/genprops/gen_props.go

@@ -198,8 +198,7 @@ func main() {
 						continue
 					}
 					qname := pkg.Types.Path() + "." + ts.Name.Name
-					if qname != "github.com/cockroachdb/pebble/sstable.Properties" &&
-						qname != "github.com/cockroachdb/pebble/sstable.CommonProperties" {
+					if qname != "github.com/cockroachdb/pebble/sstable.Properties" {
 						continue
 					}
 					for _, f := range st.Fields.List {

sstable/properties.go

@@ -11,7 +11,6 @@ import (
 	"slices"
 	"unsafe"
 
-	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/sstable/colblk"
 	"github.com/cockroachdb/pebble/sstable/rowblk"
 )
@@ -20,14 +19,10 @@ import (
 
 const propertiesBlockRestartInterval = math.MaxInt32
 
-// CommonProperties holds properties for either a virtual or a physical sstable. This
-// can be used by code which doesn't care to make the distinction between physical
-// and virtual sstables properties.
-//
-// NB: The values of these properties can affect correctness. For example,
-// if NumRangeKeySets == 0, but the sstable actually contains range keys, then
-// the iterators will behave incorrectly.
-type CommonProperties struct {
+// Properties holds the sstable property values. The properties are
+// automatically populated during sstable creation and load from the properties
+// meta block when an sstable is opened.
+type Properties struct {
 	// The number of entries in this table.
 	NumEntries uint64 `prop:"rocksdb.num.entries" options:"encodeempty"`
 	// Total raw key size.
@@ -71,30 +66,6 @@ type CommonProperties struct {
 	// This statistic is used to determine eligibility for a tombstone density
 	// compaction.
 	NumTombstoneDenseBlocks uint64 `prop:"pebble.num.tombstone-dense-blocks"`
-}
-
-// String is only used for testing purposes.
-func (c *CommonProperties) String() string {
-	p := Properties{
-		CommonProperties: *c,
-	}
-	return p.String()
-}
-
-// NumPointDeletions is the number of point deletions in the sstable. For virtual
-// sstables, this is an estimate.
-func (c *CommonProperties) NumPointDeletions() uint64 {
-	return invariants.SafeSub(c.NumDeletions, c.NumRangeDeletions)
-}
-
-// Properties holds the sstable property values. The properties are
-// automatically populated during sstable creation and load from the properties
-// meta block when an sstable is opened.
-type Properties struct {
-	// CommonProperties needs to be at the top of the Properties struct so that the
-	// offsets of the fields in CommonProperties match the offsets of the embedded
-	// fields of CommonProperties in Properties.
-	CommonProperties `prop:"pebble.embbeded_common_properties"`
 
 	// The name of the comparer used in this table.
 	ComparerName string `prop:"rocksdb.comparator" options:"encodeempty,intern"`

sstable/properties_test.go

@@ -25,14 +25,12 @@ import (
 func TestPropertiesLoad(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	expected := Properties{
-		CommonProperties: CommonProperties{
-			NumEntries:        1727,
-			NumDeletions:      17,
-			NumRangeDeletions: 17,
-			RawKeySize:        23938,
-			RawValueSize:      1912,
-			NumDataBlocks:     14,
-		},
+		NumEntries:             1727,
+		NumDeletions:           17,
+		NumRangeDeletions:      17,
+		RawKeySize:             23938,
+		RawValueSize:           1912,
+		NumDataBlocks:          14,
 		ComparerName:           "leveldb.BytewiseComparator",
 		DataSize:               13913,
 		IndexSize:              325,
@@ -63,35 +61,33 @@ func TestPropertiesLoad(t *testing.T) {
 }
 
 var testProps = Properties{
-	CommonProperties: CommonProperties{
-		NumDeletions:            15,
-		NumEntries:              16,
-		NumRangeDeletions:       18,
-		NumRangeKeyDels:         19,
-		NumRangeKeySets:         20,
-		RawKeySize:              25,
-		RawValueSize:            26,
-		NumDataBlocks:           14,
-		NumTombstoneDenseBlocks: 2,
-	},
-	ComparerName:           "comparator name",
-	DataSize:               3,
-	FilterPolicyName:       "filter policy name",
-	FilterSize:             5,
-	IndexPartitions:        10,
-	IndexSize:              11,
-	IndexType:              12,
-	IsStrictObsolete:       true,
-	KeySchemaName:          "key schema name",
-	MergerName:             "merge operator name",
-	NumMergeOperands:       17,
-	NumRangeKeyUnsets:      21,
-	NumValueBlocks:         22,
-	NumValuesInValueBlocks: 23,
-	PropertyCollectorNames: "prefix collector names",
-	TopLevelIndexSize:      27,
-	CompressionName:        "compression name",
-	CompressionStats:       "Snappy:1024/2048",
+	NumDeletions:            15,
+	NumEntries:              16,
+	NumRangeDeletions:       18,
+	NumRangeKeyDels:         19,
+	NumRangeKeySets:         20,
+	RawKeySize:              25,
+	RawValueSize:            26,
+	NumDataBlocks:           14,
+	NumTombstoneDenseBlocks: 2,
+	ComparerName:            "comparator name",
+	DataSize:                3,
+	FilterPolicyName:        "filter policy name",
+	FilterSize:              5,
+	IndexPartitions:         10,
+	IndexSize:               11,
+	IndexType:               12,
+	IsStrictObsolete:        true,
+	KeySchemaName:           "key schema name",
+	MergerName:              "merge operator name",
+	NumMergeOperands:        17,
+	NumRangeKeyUnsets:       21,
+	NumValueBlocks:          22,
+	NumValuesInValueBlocks:  23,
+	PropertyCollectorNames:  "prefix collector names",
+	TopLevelIndexSize:       27,
+	CompressionName:         "compression name",
+	CompressionStats:        "Snappy:1024/2048",
 	UserProperties: map[string]string{
 		"user-prop-a": "1",
 		"user-prop-b": "2",

table_stats.go

@@ -729,7 +729,7 @@ func maybeSetStatsFromProperties(
 		// doesn't require any additional IO and since the number of point
 		// deletions in the file is low, the error introduced by this crude
 		// estimate is expected to be small.
-		avgValSize, compressionRatio := estimatePhysicalSizes(meta, &props.CommonProperties)
+		avgValSize, compressionRatio := estimatePhysicalSizes(meta, props)
 		pointEstimate = pointDeletionsBytesEstimate(backingProps, avgValSize, compressionRatio)
 	}
 
@@ -833,7 +833,7 @@ func pointDeletionsBytesEstimate(
 }
 
 func estimatePhysicalSizes(
-	tableMeta *manifest.TableMetadata, props *sstable.CommonProperties,
+	tableMeta *manifest.TableMetadata, props *sstable.Properties,
 ) (avgValLogicalSize, compressionRatio float64) {
 	// RawKeySize and RawValueSize are uncompressed totals. Scale according to
 	// the data size to account for compression, index blocks and metadata