sstable: remove GetScaledProperties

We no longer use this "bulk" scaling of properties.

Author: RaduBerinde

data_test.go

@@ -19,6 +19,7 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/cockroachdb/crlib/crhumanize"
 	"github.com/cockroachdb/crlib/crstrings"
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/errors"
@@ -1378,22 +1379,22 @@ func runSSTablePropertiesCmd(t *testing.T, td *datadriven.TestData, d *DB) strin
 	}
 	defer r.Close()
 
-	loadedProps, err := r.ReadPropertiesBlock(context.Background(), nil /* buffer pool */)
+	props, err := r.ReadPropertiesBlock(context.Background(), nil /* buffer pool */)
 	if err != nil {
 		return err.Error()
 	}
-	props := loadedProps.String()
-	env := sstable.ReadEnv{}
+	var buf bytes.Buffer
+
 	if m != nil && m.Virtual {
-		env.Virtual = m.VirtualParams
-		scaledProps := loadedProps.GetScaledProperties(m.TableBacking.Size, m.Size)
-		props = scaledProps.String()
+		fmt.Fprintf(&buf, "virtual table (estimated size %s / backing size %s)\n",
+			crhumanize.Bytes(m.Size), crhumanize.Bytes(m.TableBacking.Size),
+		)
 	}
 	if len(td.Input) == 0 {
-		return props
+		buf.WriteString(props.String())
+		return buf.String()
 	}
-	var buf bytes.Buffer
-	propsSlice := strings.Split(props, "\n")
+	propsSlice := strings.Split(props.String(), "\n")
 	for _, requestedProp := range strings.Split(td.Input, "\n") {
 		fmt.Fprintf(&buf, "%s:\n", requestedProp)
 		for _, prop := range propsSlice {

db.go

@@ -2278,6 +2278,9 @@ type SSTableInfo struct {
 	// BackingSSTNum is the disk file number associated with the backing sstable.
 	// If Virtual is false, BackingSSTNum == PhysicalTableDiskFileNum(TableNum).
 	BackingSSTNum base.DiskFileNum
+	// BackingSize is the size of the backing sstable. This is the same with
+	// TableInfo.Size when the table is not virtual.
+	BackingSize uint64
 	// BackingType is the type of storage backing this sstable.
 	BackingType BackingType
 	// Locator is the remote.Locator backing this sstable, if the backing type is
@@ -2345,14 +2348,11 @@ func (d *DB) SSTables(opts ...SSTablesOption) ([][]SSTableInfo, error) {
 				if err != nil {
 					return nil, err
 				}
-				if m.Virtual {
-					commonProps := p.GetScaledProperties(m.TableBacking.Size, m.Size)
-					p = &sstable.Properties{CommonProperties: commonProps}
-				}
 				destTables[j].Properties = p
 			}
 			destTables[j].Virtual = m.Virtual
 			destTables[j].BackingSSTNum = m.TableBacking.DiskFileNum
+			destTables[j].BackingSize = m.TableBacking.Size
 			objMeta, err := d.objProvider.Lookup(base.FileTypeTable, m.TableBacking.DiskFileNum)
 			if err != nil {
 				return nil, err

db_test.go

@@ -1438,7 +1438,11 @@ func TestVirtualSSTables(t *testing.T) {
 	// Get the original properties.
 	tableInfos, err := d.SSTables(WithProperties())
 	require.NoError(t, err)
-	originalProps := tableInfos[0][0].Properties
+	for _, levelTables := range tableInfos {
+		for _, info := range levelTables {
+			require.NotNil(t, info.Properties)
+		}
+	}
 
 	// Excise to create virtual ssts.
 	exciseSpan := KeyRange{
@@ -1450,20 +1454,14 @@ func TestVirtualSSTables(t *testing.T) {
 
 	tableInfos, err = d.SSTables(WithProperties())
 	require.NoError(t, err)
-
-	// Find the virtual sstables and ensure that some of its common properties
-	// have been scaled down correctly.
+	sawVirtual := false
 	for _, levelTables := range tableInfos {
 		for _, info := range levelTables {
-			if info.Virtual {
-				virtualProps := info.Properties
-				require.NotNil(t, virtualProps)
-				require.Less(t, virtualProps.NumEntries, originalProps.NumEntries)
-				require.Less(t, virtualProps.RawKeySize, originalProps.RawKeySize)
-				require.Less(t, virtualProps.RawValueSize, originalProps.RawValueSize)
-			}
+			sawVirtual = sawVirtual || info.Virtual
+			require.NotNil(t, info.Properties)
 		}
 	}
+	require.True(t, sawVirtual)
 }
 
 type testTracer struct {

internal/manifest/table_metadata.go

@@ -1239,7 +1239,9 @@ func (ikr *InternalKeyBounds) SetLargest(ik InternalKey) {
 type TableInfo struct {
 	// FileNum is the internal DB identifier for the table.
 	FileNum base.FileNum
-	// Size is the size of the file in bytes.
+	// Size is the size of the table in bytes. If the table is physical, this is
+	// the same with the backing size. If the table is virtual, this is an
+	// estimation of how much data the virtual table references.
 	Size uint64
 	// Smallest is the smallest internal key in the table.
 	Smallest InternalKey

internal/manifest/table_metadata_test.go

@@ -138,6 +138,49 @@ func TestTableMetadata_ParseRoundTrip(t *testing.T) {
 	}
 }
 
+func TestTableMetadata_ScaleStatistic(t *testing.T) {
+	var meta TableMetadata
+	meta.TableBacking = &TableBacking{}
+	meta.Virtual = true
+
+	meta.Size = 50
+	meta.TableBacking.Size = 100
+	require.Equal(t, uint64(0), meta.ScaleStatistic(0))
+	require.Equal(t, uint64(1), meta.ScaleStatistic(1))
+	require.Equal(t, uint64(1), meta.ScaleStatistic(2))
+	require.Equal(t, uint64(2), meta.ScaleStatistic(3))
+	require.Equal(t, uint64(50_000), meta.ScaleStatistic(100_000))
+
+	t.Run("invalid sizes", func(t *testing.T) {
+		meta.Size = 0
+		meta.TableBacking.Size = 100
+		require.Equal(t, uint64(0), meta.ScaleStatistic(0))
+		require.Equal(t, uint64(1), meta.ScaleStatistic(1))
+		require.Equal(t, uint64(1), meta.ScaleStatistic(10))
+
+		meta.Size = 1000
+		meta.TableBacking.Size = 1000
+		require.Equal(t, uint64(0), meta.ScaleStatistic(0))
+		require.Equal(t, uint64(1), meta.ScaleStatistic(1))
+		require.Equal(t, uint64(10), meta.ScaleStatistic(10))
+	})
+
+	t.Run("overflow", func(t *testing.T) {
+		meta.Size = 1 << 60
+		meta.TableBacking.Size = 1 << 60
+		require.Equal(t, uint64(0), meta.ScaleStatistic(0))
+		require.Equal(t, uint64(1), meta.ScaleStatistic(1))
+		require.Equal(t, uint64(12345), meta.ScaleStatistic(12345))
+		require.Equal(t, uint64(1<<40), meta.ScaleStatistic(1<<40))
+		require.Equal(t, uint64(1<<50), meta.ScaleStatistic(1<<50))
+
+		meta.Size = 1 << 40
+		meta.TableBacking.Size = 1 << 50
+		require.Equal(t, uint64(1<<30), meta.ScaleStatistic(1<<40))
+		require.Equal(t, uint64(1<<40), meta.ScaleStatistic(1<<50))
+	})
+}
+
 // TestTableMetadataSize tests the expected size of our TableMetadata and
 // TableBacking structs.
 //

sstable/properties.go

@@ -11,7 +11,6 @@ import (
 	"slices"
 	"unsafe"
 
-	"github.com/cockroachdb/crlib/crmath"
 	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/sstable/colblk"
 	"github.com/cockroachdb/pebble/sstable/rowblk"
@@ -25,9 +24,6 @@ const propertiesBlockRestartInterval = math.MaxInt32
 // can be used by code which doesn't care to make the distinction between physical
 // and virtual sstables properties.
 //
-// For virtual sstables, fields are constructed through extrapolation upon virtual
-// reader construction.
-//
 // 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.
@@ -91,47 +87,6 @@ func (c *CommonProperties) NumPointDeletions() uint64 {
 	return invariants.SafeSub(c.NumDeletions, c.NumRangeDeletions)
 }
 
-// GetScaledProperties returns an estimation of the common properties for a
-// virtual table that addresses only <size> bytes out of the entire <backingSize>.
-func (c *CommonProperties) GetScaledProperties(backingSize, size uint64) CommonProperties {
-	// Make sure the sizes are sane, just in case.
-	size = max(size, 1)
-	backingSize = max(backingSize, size)
-
-	scale := func(a uint64) uint64 {
-		return crmath.ScaleUint64(a, size, backingSize)
-	}
-	// It's important that no non-zero fields (like NumDeletions, NumRangeKeySets)
-	// become zero (or vice-versa).
-	if invariants.Enabled && (scale(1) != 1 || scale(0) != 0) {
-		panic("bad scale()")
-	}
-
-	scaled := *c
-	scaled.RawKeySize = scale(c.RawKeySize)
-	scaled.RawValueSize = scale(c.RawValueSize)
-	scaled.NumEntries = scale(c.NumEntries)
-	scaled.NumDataBlocks = scale(c.NumDataBlocks)
-	scaled.NumTombstoneDenseBlocks = scale(c.NumTombstoneDenseBlocks)
-
-	scaled.NumRangeDeletions = scale(c.NumRangeDeletions)
-	scaled.NumSizedDeletions = scale(c.NumSizedDeletions)
-	// We cannot directly scale NumDeletions, because it is supposed to be the sum
-	// of various types of deletions. See #4670.
-	numOtherDeletions := scale(invariants.SafeSub(c.NumDeletions, c.NumRangeDeletions) + c.NumSizedDeletions)
-	scaled.NumDeletions = numOtherDeletions + scaled.NumRangeDeletions + scaled.NumSizedDeletions
-
-	scaled.NumRangeKeyDels = scale(c.NumRangeKeyDels)
-	scaled.NumRangeKeySets = scale(c.NumRangeKeySets)
-
-	scaled.ValueBlocksSize = scale(c.ValueBlocksSize)
-
-	scaled.RawPointTombstoneKeySize = scale(c.RawPointTombstoneKeySize)
-	scaled.RawPointTombstoneValueSize = scale(c.RawPointTombstoneValueSize)
-
-	return scaled
-}
-
 // 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.

sstable/properties_test.go

@@ -133,42 +133,6 @@ func TestPropertiesSave(t *testing.T) {
 	}
 }
 
-func TestScalePropertiesBadSizes(t *testing.T) {
-	// Verify that GetScaledProperties works even if the given sizes are not
-	// valid.
-	p := CommonProperties{
-		NumDeletions:  0,
-		NumEntries:    1,
-		NumDataBlocks: 10,
-	}
-	scaled := p.GetScaledProperties(100, 0)
-	require.Equal(t, uint64(0), scaled.NumDeletions)
-	require.Equal(t, uint64(1), scaled.NumEntries)
-	require.Equal(t, uint64(1), scaled.NumDataBlocks)
-
-	scaled = p.GetScaledProperties(100, 1000)
-	require.Equal(t, uint64(0), scaled.NumDeletions)
-	require.Equal(t, uint64(1), scaled.NumEntries)
-	require.Equal(t, uint64(10), scaled.NumDataBlocks)
-}
-
-func TestScalePropertiesOverflow(t *testing.T) {
-	// Verify that GetScaledProperties works even if the given sizes are not
-	// valid.
-	p := CommonProperties{
-		RawKeySize:   1 << 40,
-		RawValueSize: 1 << 50,
-	}
-	scaled := p.GetScaledProperties(1<<60, 1<<60)
-	require.Equal(t, p, scaled)
-
-	scaled = p.GetScaledProperties(1<<60, 1<<50)
-	require.Equal(t, scaled, CommonProperties{
-		RawKeySize:   1 << 30,
-		RawValueSize: 1 << 40,
-	})
-}
-
 func BenchmarkPropertiesLoad(b *testing.B) {
 	var w rowblk.Writer
 	w.RestartInterval = propertiesBlockRestartInterval

sstable/reader_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cockroachdb/crlib/crhumanize"
 	"github.com/cockroachdb/crlib/testutils/leaktest"
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/errors"
@@ -138,18 +139,11 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i
 		}
 	}()
 
-	formatVirtualReader := func(r *Reader, showProps bool, env ReadEnv, backingSize, tableSize uint64) string {
+	formatVirtualReader := func(r *Reader, showSize bool, env ReadEnv, backingSize, tableSize uint64) string {
 		var b bytes.Buffer
 		fmt.Fprintf(&b, "bounds:  [%s-%s]\n", env.Virtual.Lower, env.Virtual.Upper)
-		if showProps {
-			fmt.Fprintf(&b, "filenum: %s\n", env.Virtual.FileNum.String())
-			fmt.Fprintf(&b, "props:\n")
-			props, err := r.ReadPropertiesBlock(context.Background(), nil)
-			require.NoError(t, err)
-			p := props.GetScaledProperties(backingSize, tableSize)
-			for _, line := range strings.Split(strings.TrimSpace(p.String()), "\n") {
-				fmt.Fprintf(&b, "  %s\n", line)
-			}
+		if showSize {
+			fmt.Fprintf(&b, "size: %s / backing size: %s\n", crhumanize.Bytes(tableSize), crhumanize.Bytes(backingSize))
 		}
 		return b.String()
 	}
@@ -198,7 +192,7 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i
 			params.Lower = base.ParseInternalKey(lowerStr)
 			params.Upper = base.ParseInternalKey(upperStr)
 
-			showProps := td.HasArg("show-props")
+			showSize := td.HasArg("show-size")
 
 			var syntheticPrefix []byte
 			if td.HasArg("prefix") {
@@ -225,7 +219,7 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i
 			if err != nil {
 				return err.Error()
 			}
-			return formatVirtualReader(r, showProps, env, wMeta.Size, tableSize)
+			return formatVirtualReader(r, showSize, env, wMeta.Size, tableSize)
 
 		case "compaction-iter":
 			// Creates a compaction iterator from the virtual reader, and then