sstable: show filenum when metadata is corrupted

Fixes #3804.
cockroachdb · Jul 29, 2024 · 368636f · 368636f
1 parent f8e117d
commit 368636f
Show file tree

Hide file tree

Showing 11 changed files with 37 additions and 23 deletions.
diff --git a/sstable/format.go b/sstable/format.go
@@ -210,14 +210,14 @@ const (
 
 // ParseTableFormat parses the given magic bytes and version into its
 // corresponding internal TableFormat.
-func ParseTableFormat(magic []byte, version uint32) (TableFormat, error) {
+func ParseTableFormat(magic []byte, version uint32, fileNum base.DiskFileNum) (TableFormat, error) {
 	switch string(magic) {
 	case levelDBMagic:
 		return TableFormatLevelDB, nil
 	case rocksDBMagic:
 		if version != rocksDBFormatVersion2 {
 			return TableFormatUnspecified, base.CorruptionErrorf(
-				"pebble/table: unsupported rocksdb format version %d", errors.Safe(version),
+				"pebble/table: invalid table %s (unsupported rocksdb format version %d)", errors.Safe(fileNum), errors.Safe(version),
 			)
 		}
 		return TableFormatRocksDBv2, nil
@@ -233,12 +233,12 @@ func ParseTableFormat(magic []byte, version uint32) (TableFormat, error) {
 			return TableFormatPebblev4, nil
 		default:
 			return TableFormatUnspecified, base.CorruptionErrorf(
-				"pebble/table: unsupported pebble format version %d", errors.Safe(version),
+				"pebble/table: invalid table %s (unsupported pebble format version %d)", errors.Safe(fileNum), errors.Safe(version),
 			)
 		}
 	default:
 		return TableFormatUnspecified, base.CorruptionErrorf(
-			"pebble/table: invalid table (bad magic number: 0x%x)", magic,
+			"pebble/table: invalid table %s (bad magic number: 0x%x)", errors.Safe(fileNum), magic,
 		)
 	}
 }

diff --git a/sstable/format_test.go b/sstable/format_test.go
@@ -60,25 +60,25 @@ func TestTableFormat_RoundTrip(t *testing.T) {
 			name:    "Invalid RocksDB version",
 			magic:   rocksDBMagic,
 			version: 1,
-			wantErr: "pebble/table: unsupported rocksdb format version 1",
+			wantErr: "pebble/table: invalid table 000001 (unsupported rocksdb format version 1)",
 		},
 		{
 			name:    "Invalid PebbleDB version",
 			magic:   pebbleDBMagic,
 			version: 5,
-			wantErr: "pebble/table: unsupported pebble format version 5",
+			wantErr: "pebble/table: invalid table 000001 (unsupported pebble format version 5)",
 		},
 		{
 			name:    "Unknown magic string",
 			magic:   "foo",
-			wantErr: "pebble/table: invalid table (bad magic number: 0x666f6f)",
+			wantErr: "pebble/table: invalid table 000001 (bad magic number: 0x666f6f)",
 		},
 	}
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
 			// Tuple -> TableFormat.
-			f, err := ParseTableFormat([]byte(tc.magic), tc.version)
+			f, err := ParseTableFormat([]byte(tc.magic), tc.version, 1)
 			if tc.wantErr != "" {
 				require.Error(t, err)
 				require.Equal(t, tc.wantErr, err.Error())

diff --git a/sstable/reader.go b/sstable/reader.go
@@ -979,7 +979,7 @@ func NewReader(ctx context.Context, f objstorage.Readable, o ReaderOptions) (*Re
 		r.readable, objstorage.ReadBeforeForNewReader, &preallocRH)
 	defer rh.Close()
 
-	footer, err := readFooter(ctx, f, rh, r.logger)
+	footer, err := readFooter(ctx, f, rh, r.logger, r.cacheOpts.FileNum)
 	if err != nil {
 		r.err = err
 		return nil, r.Close()

diff --git a/sstable/table.go b/sstable/table.go
@@ -307,11 +307,12 @@ func readFooter(
 	f objstorage.Readable,
 	readHandle objstorage.ReadHandle,
 	logger base.LoggerAndTracer,
+	fileNum base.DiskFileNum,
 ) (footer, error) {
 	var footer footer
 	size := f.Size()
 	if size < minFooterLen {
-		return footer, base.CorruptionErrorf("pebble/table: invalid table (file size is too small)")
+		return footer, base.CorruptionErrorf("pebble/table: invalid table %s (file size is too small)", errors.Safe(fileNum))
 	}
 
 	buf := make([]byte, maxFooterLen)
@@ -342,7 +343,7 @@ func readFooter(
 	case levelDBMagic:
 		if len(buf) < levelDBFooterLen {
 			return footer, base.CorruptionErrorf(
-				"pebble/table: invalid table (footer too short): %d", errors.Safe(len(buf)))
+				"pebble/table: invalid table %s (footer too short): %d", errors.Safe(fileNum), errors.Safe(len(buf)))
 		}
 		footer.footerBH.Offset = uint64(off+int64(len(buf))) - levelDBFooterLen
 		buf = buf[len(buf)-levelDBFooterLen:]
@@ -354,14 +355,14 @@ func readFooter(
 		// NOTE: The Pebble magic string implies the same footer format as that used
 		// by the RocksDBv2 table format.
 		if len(buf) < rocksDBFooterLen {
-			return footer, base.CorruptionErrorf("pebble/table: invalid table (footer too short): %d", errors.Safe(len(buf)))
+			return footer, base.CorruptionErrorf("pebble/table: invalid table %s (footer too short): %d", errors.Safe(fileNum), errors.Safe(len(buf)))
 		}
 		footer.footerBH.Offset = uint64(off+int64(len(buf))) - rocksDBFooterLen
 		buf = buf[len(buf)-rocksDBFooterLen:]
 		footer.footerBH.Length = uint64(len(buf))
 		version := binary.LittleEndian.Uint32(buf[rocksDBVersionOffset:rocksDBMagicOffset])
 
-		format, err := ParseTableFormat(magic, version)
+		format, err := ParseTableFormat(magic, version, fileNum)
 		if err != nil {
 			return footer, err
 		}
@@ -373,26 +374,26 @@ func readFooter(
 		case block.ChecksumTypeXXHash64:
 			footer.checksum = block.ChecksumTypeXXHash64
 		default:
-			return footer, base.CorruptionErrorf("pebble/table: unsupported checksum type %d", errors.Safe(footer.checksum))
+			return footer, base.CorruptionErrorf("pebble/table: invalid table %s (unsupported checksum type %d)", errors.Safe(fileNum), errors.Safe(footer.checksum))
 		}
 		buf = buf[1:]
 
 	default:
-		return footer, base.CorruptionErrorf("pebble/table: invalid table (bad magic number: 0x%x)", magic)
+		return footer, base.CorruptionErrorf("pebble/table: invalid table %s (bad magic number: 0x%x)", errors.Safe(fileNum), magic)
 	}
 
 	{
 		end := uint64(size)
 		var n int
 		footer.metaindexBH, n = decodeBlockHandle(buf)
 		if n == 0 || footer.metaindexBH.Offset+footer.metaindexBH.Length > end {
-			return footer, base.CorruptionErrorf("pebble/table: invalid table (bad metaindex block handle)")
+			return footer, base.CorruptionErrorf("pebble/table: invalid table %s (bad metaindex block handle)", errors.Safe(fileNum))
 		}
 		buf = buf[n:]
 
 		footer.indexBH, n = decodeBlockHandle(buf)
 		if n == 0 || footer.indexBH.Offset+footer.indexBH.Length > end {
-			return footer, base.CorruptionErrorf("pebble/table: invalid table (bad index block handle)")
+			return footer, base.CorruptionErrorf("pebble/table: invalid table %s (bad index block handle)", errors.Safe(fileNum))
 		}
 	}
 

diff --git a/sstable/table_test.go b/sstable/table_test.go
@@ -588,7 +588,7 @@ func TestFooterRoundTrip(t *testing.T) {
 							readable, err := NewSimpleReadable(f)
 							require.NoError(t, err)
 
-							result, err := readFooter(context.Background(), readable, nil, base.NoopLoggerAndTracer{})
+							result, err := readFooter(context.Background(), readable, nil, base.NoopLoggerAndTracer{}, 1)
 							require.NoError(t, err)
 							require.NoError(t, readable.Close())
 
@@ -641,10 +641,12 @@ func TestReadFooter(t *testing.T) {
 			readable, err := NewSimpleReadable(f)
 			require.NoError(t, err)
 
-			if _, err := readFooter(context.Background(), readable, nil, base.NoopLoggerAndTracer{}); err == nil {
+			if _, err := readFooter(context.Background(), readable, nil, base.NoopLoggerAndTracer{}, 1); err == nil {
 				t.Fatalf("expected %q, but found success", c.expected)
 			} else if !strings.Contains(err.Error(), c.expected) {
 				t.Fatalf("expected %q, but found %v", c.expected, err)
+			} else if !strings.Contains(err.Error(), "table 000001") {
+				t.Fatalf("expected error to contain table number: %q", err)
 			}
 		})
 	}

diff --git a/table_cache_test.go b/table_cache_test.go
@@ -1025,7 +1025,7 @@ func TestTableCacheErrorBadMagicNumber(t *testing.T) {
 		t.Fatalf("expected failure, but found success")
 	}
 	require.Equal(t,
-		"pebble: backing file 000003 error: pebble/table: invalid table (bad magic number: 0xf09faab3f09faa00)",
+		"pebble: backing file 000003 error: pebble/table: invalid table 000003 (bad magic number: 0xf09faab3f09faa00)",
 		err.Error())
 }
 

diff --git a/tool/db.go b/tool/db.go
@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/humanize"
 	"github.com/cockroachdb/pebble/internal/manifest"
+	"github.com/cockroachdb/pebble/internal/sstableinternal"
 	"github.com/cockroachdb/pebble/objstorage"
 	"github.com/cockroachdb/pebble/objstorage/objstorageprovider"
 	"github.com/cockroachdb/pebble/record"
@@ -883,6 +884,15 @@ func (d *dbT) addProps(
 	if err != nil {
 		return err
 	}
+	opts := sstable.ReaderOptions{
+		Mergers:   d.mergers,
+		Comparers: d.comparers,
+	}
+	opts.SetInternal(sstableinternal.ReaderOptions{
+		CacheOpts: sstableinternal.CacheOptions{
+			FileNum: m.FileBacking.DiskFileNum,
+		},
+	})
 	r, err := sstable.NewReader(ctx, f, sstable.ReaderOptions{
 		Mergers:   d.mergers,
 		Comparers: d.comparers,

diff --git a/tool/find.go b/tool/find.go
@@ -446,7 +446,8 @@ func (f *findT) searchTables(stdout io.Writer, searchKey []byte, refs []findRef)
 			}
 			opts.SetInternal(sstableinternal.ReaderOptions{
 				CacheOpts: sstableinternal.CacheOptions{
-					Cache: cache,
+					Cache:   cache,
+					FileNum: fl.DiskFileNum,
 				},
 			})
 			readable, err := sstable.NewSimpleReadable(tf)

diff --git a/tool/testdata/find b/tool/testdata/find
@@ -137,4 +137,4 @@ hex:636363
 ----
 000002.sst
     test formatter: ccc#14,MERGE
-Unable to decode sstable find-mixed/000001.sst, pebble/table: invalid table (file size is too small)
+Unable to decode sstable find-mixed/000001.sst, pebble/table: invalid table 000001 (file size is too small)
diff --git a/tool/testdata/sstable_check b/tool/testdata/sstable_check
@@ -51,7 +51,7 @@ sstable check
 testdata/bad-magic.sst
 ----
 bad-magic.sst
-pebble/table: invalid table (bad magic number: 0xf6cff485b741e288)
+pebble/table: invalid table 000000 (bad magic number: 0xf6cff485b741e288)
 
 sstable check
 ./testdata/mixed/000005.sst

diff --git a/tool/testdata/sstable_properties b/tool/testdata/sstable_properties