From 5c1d05640850b49dadfb89c7a077a5dff01024c2 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Thu, 14 Mar 2024 09:28:43 -0700 Subject: [PATCH] remove support for prefix replacement Remove `PrefixReplacement` in favor of `SyntheticPrefix`. The general prefix replacement turns out to be too difficult so we will instead strip the prefix when writing out the backup files. --- compaction.go | 16 +- data_test.go | 7 +- ingest.go | 17 +- internal/manifest/version.go | 40 ++- internal/manifest/version_edit.go | 32 +- internal/manifest/version_edit_test.go | 16 +- lsm_view.go | 7 +- metamorphic/build.go | 28 +- metamorphic/generator.go | 26 +- metamorphic/key_manager.go | 4 +- metamorphic/ops.go | 25 +- metamorphic/parser.go | 14 +- scan_internal.go | 5 +- sstable/block.go | 6 +- sstable/prefix_replacing_iterator.go | 368 ---------------------- sstable/prefix_replacing_iterator_test.go | 138 -------- sstable/reader.go | 18 ++ sstable/reader_common.go | 37 --- sstable/reader_iter_single_lvl.go | 16 +- sstable/reader_virtual.go | 54 +--- testdata/ingest_external | 113 +------ 21 files changed, 117 insertions(+), 870 deletions(-) delete mode 100644 sstable/prefix_replacing_iterator.go delete mode 100644 sstable/prefix_replacing_iterator_test.go diff --git a/compaction.go b/compaction.go index b5b4fc3257..2748575306 100644 --- a/compaction.go +++ b/compaction.go @@ -2444,14 +2444,14 @@ func (d *DB) runCopyCompaction( // a new FileNum. This has the potential of making the block cache less // effective, however. newMeta := &fileMetadata{ - Size: inputMeta.Size, - CreationTime: inputMeta.CreationTime, - SmallestSeqNum: inputMeta.SmallestSeqNum, - LargestSeqNum: inputMeta.LargestSeqNum, - Stats: inputMeta.Stats, - PrefixReplacement: inputMeta.PrefixReplacement, - Virtual: inputMeta.Virtual, - SyntheticSuffix: inputMeta.SyntheticSuffix, + Size: inputMeta.Size, + CreationTime: inputMeta.CreationTime, + SmallestSeqNum: inputMeta.SmallestSeqNum, + LargestSeqNum: inputMeta.LargestSeqNum, + Stats: inputMeta.Stats, + Virtual: inputMeta.Virtual, + SyntheticPrefix: inputMeta.SyntheticPrefix, + SyntheticSuffix: inputMeta.SyntheticSuffix, } if inputMeta.HasPointKeys { newMeta.ExtendPointKeyBounds(c.cmp, inputMeta.SmallestPointKey, inputMeta.LargestPointKey) diff --git a/data_test.go b/data_test.go index 1f52938b24..7c9cdb8329 100644 --- a/data_test.go +++ b/data_test.go @@ -1294,7 +1294,7 @@ func runIngestExternalCmd( usageErr := func(info interface{}) { t.Helper() td.Fatalf(t, "error parsing %q: %v; "+ - "usage: obj bounds=(smallest,largest) [size=x] [prefix-replace=(from,to)] [synthetic-prefix=prefix] [synthetic-suffix=suffix]", + "usage: obj bounds=(smallest,largest) [size=x] [synthetic-prefix=prefix] [synthetic-suffix=suffix]", line, info, ) } @@ -1328,11 +1328,6 @@ func runIngestExternalCmd( nArgs(1) arg.Scan(t, 0, &ef.Size) - case "prefix-replace": - nArgs(2) - ef.ContentPrefix = []byte(arg.Vals[0]) - ef.SyntheticPrefix = []byte(arg.Vals[1]) - case "synthetic-prefix": nArgs(1) ef.SyntheticPrefix = []byte(arg.Vals[0]) diff --git a/ingest.go b/ingest.go index 42ff0f2294..852b820a53 100644 --- a/ingest.go +++ b/ingest.go @@ -232,12 +232,7 @@ func ingestLoad1External( ) } - if len(e.SyntheticPrefix) != 0 { - meta.PrefixReplacement = &sstable.PrefixReplacement{ - ContentPrefix: e.ContentPrefix, - SyntheticPrefix: e.SyntheticPrefix, - } - } + meta.SyntheticPrefix = e.SyntheticPrefix meta.SyntheticSuffix = e.SyntheticSuffix if err := meta.Validate(opts.Comparer.Compare, opts.Comparer.FormatKey); err != nil { @@ -1153,15 +1148,11 @@ type ExternalFile struct { // ingestion. HasPointKey, HasRangeKey bool - // ContentPrefix and SyntheticPrefix denote a prefix replacement rule causing - // a file, in which all keys have prefix ContentPrefix, to appear whenever it - // is accessed as if those keys all instead have prefix SyntheticPrefix. + // SyntheticPrefix will prepend this suffix to all keys in the file during + // iteration. Note that the backing file itself is not modified. // // SyntheticPrefix must be a prefix of both Bounds.Start and Bounds.End. - // - // NB: If the SyntheticPrefix is non-empty and the ContentPrefix is empty, - // then the read path will conduct block level prefix synthesis. - ContentPrefix, SyntheticPrefix []byte + SyntheticPrefix []byte // SyntheticSuffix will replace the suffix of every key in the file during // iteration. Note that the file itself is not modified, rather, every key diff --git a/internal/manifest/version.go b/internal/manifest/version.go index 41f917b6e7..4fbb4873b8 100644 --- a/internal/manifest/version.go +++ b/internal/manifest/version.go @@ -270,10 +270,11 @@ type FileMetadata struct { // Virtual is true if the FileMetadata belongs to a virtual sstable. Virtual bool - // PrefixReplacement is used for virtual files where the backing file has a - // different prefix on its keys than the span in which it is being exposed. - PrefixReplacement *sstable.PrefixReplacement + // SyntheticPrefix is used to prepend a prefix to all keys; used for some virtual + // tables. + SyntheticPrefix sstable.SyntheticPrefix + // SyntheticSuffix overrides all suffixes in a table; used for some virtual tables. SyntheticSuffix sstable.SyntheticSuffix } @@ -293,14 +294,10 @@ func (m *FileMetadata) SyntheticSeqNum() sstable.SyntheticSeqNum { // IterTransforms returns an sstable.IterTransforms that has SyntheticSeqNum set as needed. func (m *FileMetadata) IterTransforms() sstable.IterTransforms { - var syntheticPrefix []byte - if m.PrefixReplacement != nil && !m.PrefixReplacement.UsePrefixReplacementIterator() { - syntheticPrefix = m.PrefixReplacement.SyntheticPrefix - } return sstable.IterTransforms{ SyntheticSeqNum: m.SyntheticSeqNum(), SyntheticSuffix: m.SyntheticSuffix, - SyntheticPrefix: syntheticPrefix, + SyntheticPrefix: m.SyntheticPrefix, } } @@ -353,13 +350,12 @@ type VirtualFileMeta struct { // sstable reader. func (m VirtualFileMeta) VirtualReaderParams(isShared bool) sstable.VirtualReaderParams { return sstable.VirtualReaderParams{ - Lower: m.Smallest, - Upper: m.Largest, - FileNum: m.FileNum, - IsSharedIngested: isShared && m.SyntheticSeqNum() != 0, - Size: m.Size, - BackingSize: m.FileBacking.Size, - PrefixReplacement: m.PrefixReplacement, + Lower: m.Smallest, + Upper: m.Largest, + FileNum: m.FileNum, + IsSharedIngested: isShared && m.SyntheticSeqNum() != 0, + Size: m.Size, + BackingSize: m.FileBacking.Size, } } @@ -876,21 +872,21 @@ func (m *FileMetadata) Validate(cmp Compare, formatKey base.FormatKey) error { return base.CorruptionErrorf("file metadata FileBacking not set") } - if m.PrefixReplacement != nil { + if m.SyntheticPrefix.IsSet() { if !m.Virtual { - return base.CorruptionErrorf("prefix replacement rule set with non-virtual file") + return base.CorruptionErrorf("non-virtual file with synthetic prefix") } - if !bytes.HasPrefix(m.Smallest.UserKey, m.PrefixReplacement.SyntheticPrefix) { - return base.CorruptionErrorf("virtual file with prefix replacement rules has smallest key with a different prefix: %s", m.Smallest.Pretty(formatKey)) + if !bytes.HasPrefix(m.Smallest.UserKey, m.SyntheticPrefix) { + return base.CorruptionErrorf("virtual file with synthetic prefix has smallest key with a different prefix: %s", m.Smallest.Pretty(formatKey)) } - if !bytes.HasPrefix(m.Largest.UserKey, m.PrefixReplacement.SyntheticPrefix) { - return base.CorruptionErrorf("virtual file with prefix replacement rules has largest key with a different prefix: %s", m.Largest.Pretty(formatKey)) + if !bytes.HasPrefix(m.Largest.UserKey, m.SyntheticPrefix) { + return base.CorruptionErrorf("virtual file with synthetic prefix has largest key with a different prefix: %s", m.Largest.Pretty(formatKey)) } } if m.SyntheticSuffix != nil { if !m.Virtual { - return base.CorruptionErrorf("suffix replacement rule set with non-virtual file") + return base.CorruptionErrorf("non-virtual file with synthetic suffix") } } diff --git a/internal/manifest/version_edit.go b/internal/manifest/version_edit.go index f7a5be41dc..95d0bfa2f5 100644 --- a/internal/manifest/version_edit.go +++ b/internal/manifest/version_edit.go @@ -67,8 +67,8 @@ const ( customTagPathID = 65 customTagNonSafeIgnoreMask = 1 << 6 customTagVirtual = 66 - customTagPrefixRewrite = 67 - customTagSuffixRewrite = 68 + customTagSyntheticPrefix = 67 + customTagSyntheticSuffix = 68 ) // DeletedFileEntry holds the state for a file deletion from a level. The file @@ -342,8 +342,8 @@ func (v *VersionEdit) Decode(r io.Reader) error { virtual bool backingFileNum uint64 }{} - var virtualPrefix *sstable.PrefixReplacement - var syntheticSuffix []byte + var syntheticPrefix sstable.SyntheticPrefix + var syntheticSuffix sstable.SyntheticSuffix if tag == tagNewFile4 || tag == tagNewFile5 { for { customTag, err := d.readUvarint() @@ -384,21 +384,14 @@ func (v *VersionEdit) Decode(r io.Reader) error { return err } - case customTagPrefixRewrite: - content, err := d.readBytes() - if err != nil { - return err - } + case customTagSyntheticPrefix: synthetic, err := d.readBytes() if err != nil { return err } - virtualPrefix = &sstable.PrefixReplacement{ - ContentPrefix: content, - SyntheticPrefix: synthetic, - } + syntheticPrefix = synthetic - case customTagSuffixRewrite: + case customTagSyntheticSuffix: if syntheticSuffix, err = d.readBytes(); err != nil { return err } @@ -421,7 +414,7 @@ func (v *VersionEdit) Decode(r io.Reader) error { LargestSeqNum: largestSeqNum, MarkedForCompaction: markedForCompaction, Virtual: virtualState.virtual, - PrefixReplacement: virtualPrefix, + SyntheticPrefix: syntheticPrefix, SyntheticSuffix: syntheticSuffix, } if tag != tagNewFile5 { // no range keys present @@ -705,13 +698,12 @@ func (v *VersionEdit) Encode(w io.Writer) error { e.writeUvarint(customTagVirtual) e.writeUvarint(uint64(x.Meta.FileBacking.DiskFileNum)) } - if x.Meta.PrefixReplacement != nil { - e.writeUvarint(customTagPrefixRewrite) - e.writeBytes(x.Meta.PrefixReplacement.ContentPrefix) - e.writeBytes(x.Meta.PrefixReplacement.SyntheticPrefix) + if x.Meta.SyntheticPrefix != nil { + e.writeUvarint(customTagSyntheticPrefix) + e.writeBytes(x.Meta.SyntheticPrefix) } if x.Meta.SyntheticSuffix != nil { - e.writeUvarint(customTagSuffixRewrite) + e.writeUvarint(customTagSyntheticSuffix) e.writeBytes(x.Meta.SyntheticSuffix) } e.writeUvarint(customTagTerminate) diff --git a/internal/manifest/version_edit_test.go b/internal/manifest/version_edit_test.go index cf7d44a256..dff41dcacd 100644 --- a/internal/manifest/version_edit_test.go +++ b/internal/manifest/version_edit_test.go @@ -18,7 +18,6 @@ import ( "github.com/cockroachdb/errors" "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/record" - "github.com/cockroachdb/pebble/sstable" "github.com/kr/pretty" "github.com/stretchr/testify/require" ) @@ -45,15 +44,12 @@ func checkRoundTrip(e0 VersionEdit) error { func TestVERoundTripAndAccumulate(t *testing.T) { cmp := base.DefaultComparer.Compare m1 := (&FileMetadata{ - FileNum: 810, - Size: 8090, - CreationTime: 809060, - SmallestSeqNum: 9, - LargestSeqNum: 11, - PrefixReplacement: &sstable.PrefixReplacement{ - ContentPrefix: []byte("before"), - SyntheticPrefix: []byte("after"), - }, + FileNum: 810, + Size: 8090, + CreationTime: 809060, + SmallestSeqNum: 9, + LargestSeqNum: 11, + SyntheticPrefix: []byte("after"), SyntheticSuffix: []byte("foo"), }).ExtendPointKeyBounds( cmp, diff --git a/lsm_view.go b/lsm_view.go index 5349ecc22a..8c58354c82 100644 --- a/lsm_view.go +++ b/lsm_view.go @@ -181,10 +181,11 @@ func (b *lsmViewBuilder) tableDetails( outf("virtual; backed by %s (%ssize: %s)", m.FileBacking.DiskFileNum, backingInfo, humanize.Bytes.Uint64(m.FileBacking.Size)) } outf("seqnums: %d - %d", m.SmallestSeqNum, m.LargestSeqNum) - if p := m.PrefixReplacement; p != nil { - outf("prefix replacement: %s -> %s", p.ContentPrefix, p.SyntheticPrefix) + if m.SyntheticPrefix.IsSet() { + // Note: we are abusing the key formatter by passing just the prefix. + outf("synthetic prefix: %s", b.fmtKey(m.SyntheticPrefix)) } - if len(m.SyntheticSuffix) > 0 { + if m.SyntheticSuffix.IsSet() { // Note: we are abusing the key formatter by passing just the suffix. outf("synthetic suffix: %s", b.fmtKey(m.SyntheticSuffix)) } diff --git a/metamorphic/build.go b/metamorphic/build.go index e93e415137..c9e603e706 100644 --- a/metamorphic/build.go +++ b/metamorphic/build.go @@ -30,7 +30,7 @@ func writeSSTForIngestion( rangeKeyIter keyspan.FragmentIterator, uniquePrefixes bool, syntheticSuffix sstable.SyntheticSuffix, - prefixChange *sstable.PrefixReplacement, + syntheticPrefix sstable.SyntheticPrefix, writable objstorage.Writable, targetFMV pebble.FormatMajorVersion, ) (*sstable.WriterMetadata, error) { @@ -47,11 +47,11 @@ func writeSSTForIngestion( defer rangeKeyIterCloser.Close() outputKey := func(key []byte) []byte { - if prefixChange == nil && !syntheticSuffix.IsSet() { + if !syntheticPrefix.IsSet() && !syntheticSuffix.IsSet() { return slices.Clone(key) } - if prefixChange != nil { - key = prefixChange.Apply(key) + if syntheticPrefix.IsSet() { + key = syntheticPrefix.Apply(key) } if syntheticSuffix.IsSet() { n := t.opts.Comparer.Split(key) @@ -179,7 +179,7 @@ func buildForIngest( iter, rangeDelIter, rangeKeyIter, false, /* uniquePrefixes */ nil, /* syntheticSuffix */ - nil, /* prefixChange */ + nil, /* syntheticPrefix */ writable, db.FormatMajorVersion(), ) @@ -195,14 +195,14 @@ func buildForIngestExternalEmulation( externalObjID objID, bounds pebble.KeyRange, syntheticSuffix sstable.SyntheticSuffix, - prefixChange *sstable.PrefixReplacement, + syntheticPrefix sstable.SyntheticPrefix, i int, ) (path string, _ *sstable.WriterMetadata) { path = t.opts.FS.PathJoin(t.tmpDir, fmt.Sprintf("ext%d-%d", dbID.slot(), i)) f, err := t.opts.FS.Create(path) panicIfErr(err) - reader, pointIter, rangeDelIter, rangeKeyIter := openExternalObj(t, externalObjID, bounds, prefixChange) + reader, pointIter, rangeDelIter, rangeKeyIter := openExternalObj(t, externalObjID, bounds, syntheticPrefix) defer reader.Close() writable := objstorageprovider.NewFileWritable(f) @@ -215,7 +215,7 @@ func buildForIngestExternalEmulation( pointIter, rangeDelIter, rangeKeyIter, uniquePrefixes, syntheticSuffix, - prefixChange, + syntheticPrefix, writable, t.minFMV(), ) @@ -226,7 +226,7 @@ func buildForIngestExternalEmulation( } func openExternalObj( - t *Test, externalObjID objID, bounds pebble.KeyRange, prefixChange *sstable.PrefixReplacement, + t *Test, externalObjID objID, bounds pebble.KeyRange, syntheticPrefix sstable.SyntheticPrefix, ) ( reader *sstable.Reader, pointIter base.InternalIterator, @@ -243,9 +243,9 @@ func openExternalObj( start := bounds.Start end := bounds.End - if prefixChange != nil { - start = prefixChange.Invert(start) - end = prefixChange.Invert(end) + if syntheticPrefix.IsSet() { + start = syntheticPrefix.Invert(start) + end = syntheticPrefix.Invert(end) } pointIter, err = reader.NewIter(sstable.NoTransforms, start, end) panicIfErr(err) @@ -277,9 +277,9 @@ func openExternalObj( // externalObjIsEmpty returns true if the given external object has no point or // range keys withing the given bounds. func externalObjIsEmpty( - t *Test, externalObjID objID, bounds pebble.KeyRange, prefixChange *sstable.PrefixReplacement, + t *Test, externalObjID objID, bounds pebble.KeyRange, syntheticPrefix sstable.SyntheticPrefix, ) bool { - reader, pointIter, rangeDelIter, rangeKeyIter := openExternalObj(t, externalObjID, bounds, prefixChange) + reader, pointIter, rangeDelIter, rangeKeyIter := openExternalObj(t, externalObjID, bounds, syntheticPrefix) defer reader.Close() defer closeIters(pointIter, rangeDelIter, rangeKeyIter) diff --git a/metamorphic/generator.go b/metamorphic/generator.go index 6d0715a1f7..1f88ddef3c 100644 --- a/metamorphic/generator.go +++ b/metamorphic/generator.go @@ -1314,28 +1314,14 @@ func (g *generator) writerIngestExternalFiles() { if g.cmp(start, end) == 0 { end = objEnd } - // Randomly set up prefix change. - var prefixChange *sstable.PrefixReplacement + // Randomly set up synthetic prefix. + var syntheticPrefix sstable.SyntheticPrefix // We can only use a synthetic prefix if we don't have range dels. // TODO(radu): we will want to support this at some point. if !g.keyManager.objKeyMeta(id).hasRangeDels && g.rng.Intn(2) == 0 { - prefixChange = &sstable.PrefixReplacement{ - SyntheticPrefix: randBytes(g.rng, 1, 5), - } - // TODO(radu): fix prefix replacement implementation or remove - // ContentPrefix altogether. - if false { - prefixLen := 0 - limit := min(len(start), len(end)) - for ; prefixLen < limit && start[prefixLen] == end[prefixLen]; prefixLen++ { - } - prefixLen = g.rng.Intn(prefixLen + 1) - if prefixLen > 0 { - prefixChange.ContentPrefix = start[:prefixLen] - } - } - start = prefixChange.Apply(start) - end = prefixChange.Apply(end) + syntheticPrefix = randBytes(g.rng, 1, 5) + start = syntheticPrefix.Apply(start) + end = syntheticPrefix.Apply(end) } objs[i] = externalObjWithBounds{ @@ -1344,7 +1330,7 @@ func (g *generator) writerIngestExternalFiles() { Start: start, End: end, }, - prefixChange: prefixChange, + syntheticPrefix: syntheticPrefix, } } diff --git a/metamorphic/key_manager.go b/metamorphic/key_manager.go index c04fb07621..15753c7dee 100644 --- a/metamorphic/key_manager.go +++ b/metamorphic/key_manager.go @@ -302,8 +302,8 @@ func (k *keyManager) KeysForExternalIngest(obj externalObjWithBounds) []keyMeta var res []keyMeta for _, km := range k.SortedKeysForObj(obj.externalObjID) { // Apply prefix and suffix changes, then check the bounds. - if obj.prefixChange != nil { - km.key = obj.prefixChange.Apply(km.key) + if obj.syntheticPrefix.IsSet() { + km.key = obj.syntheticPrefix.Apply(km.key) } if obj.syntheticSuffix.IsSet() { n := k.comparer.Split(km.key) diff --git a/metamorphic/ops.go b/metamorphic/ops.go index 49ca80feb4..e1136dc648 100644 --- a/metamorphic/ops.go +++ b/metamorphic/ops.go @@ -930,8 +930,8 @@ type externalObjWithBounds struct { // any prefix or suffix transforms. bounds pebble.KeyRange + syntheticPrefix sstable.SyntheticPrefix syntheticSuffix sstable.SyntheticSuffix - prefixChange *sstable.PrefixReplacement } func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) { @@ -945,7 +945,7 @@ func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) { if m := objMeta.sstMeta; !m.HasPointKeys && !m.HasRangeKeys && !m.HasRangeDelKeys { continue } - if externalObjIsEmpty(t, obj.externalObjID, obj.bounds, obj.prefixChange) { + if externalObjIsEmpty(t, obj.externalObjID, obj.bounds, obj.syntheticPrefix) { // Currently we don't support ingesting external objects that have no point // or range keys in the given range. Filter out any such objects. // TODO(radu): even though we don't expect this case in practice, eventually @@ -966,7 +966,7 @@ func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) { for i, obj := range objs { // Make sure the object exists and is not empty. path, sstMeta := buildForIngestExternalEmulation( - t, o.dbID, obj.externalObjID, obj.bounds, obj.syntheticSuffix, obj.prefixChange, i, + t, o.dbID, obj.externalObjID, obj.bounds, obj.syntheticSuffix, obj.syntheticPrefix, i, ) if sstMeta.HasPointKeys || sstMeta.HasRangeKeys || sstMeta.HasRangeDelKeys { paths = append(paths, path) @@ -990,9 +990,8 @@ func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) { HasRangeKey: meta.sstMeta.HasRangeKeys, SyntheticSuffix: obj.syntheticSuffix, } - if obj.prefixChange != nil { - external[i].ContentPrefix = obj.prefixChange.ContentPrefix - external[i].SyntheticPrefix = obj.prefixChange.SyntheticPrefix + if obj.syntheticPrefix.IsSet() { + external[i].SyntheticPrefix = obj.syntheticPrefix } } _, err = db.IngestExternalFiles(external) @@ -1015,14 +1014,8 @@ func (o *ingestExternalFilesOp) syncObjs() objIDSlice { func (o *ingestExternalFilesOp) String() string { strs := make([]string, len(o.objs)) for i, obj := range o.objs { - var contentPrefix, syntheticPrefix []byte - if obj.prefixChange != nil { - contentPrefix = obj.prefixChange.ContentPrefix - syntheticPrefix = obj.prefixChange.SyntheticPrefix - } - strs[i] = fmt.Sprintf("%s, %q /* start */, %q /* end */, %q /* syntheticSuffix */, %q /* contentPrefix */, %q /* syntheticPrefix */", - obj.externalObjID, obj.bounds.Start, obj.bounds.End, obj.syntheticSuffix, - contentPrefix, syntheticPrefix, + strs[i] = fmt.Sprintf("%s, %q /* start */, %q /* end */, %q /* syntheticSuffix */, %q /* syntheticPrefix */", + obj.externalObjID, obj.bounds.Start, obj.bounds.End, obj.syntheticSuffix, obj.syntheticPrefix, ) } return fmt.Sprintf("%s.IngestExternalFiles(%s)", o.dbID, strings.Join(strs, ", ")) @@ -1032,7 +1025,7 @@ func (o *ingestExternalFilesOp) keys() []*[]byte { // If any of the objects have synthetic prefixes, we can't allow modification // of external object bounds. for i := range o.objs { - if o.objs[i].prefixChange != nil { + if o.objs[i].syntheticPrefix.IsSet() { return nil } } @@ -1788,7 +1781,7 @@ func (o *newExternalObjOp) run(t *Test, h historyRecorder) { iter, rangeDelIter, rangeKeyIter, true, /* uniquePrefixes */ nil, /* syntheticSuffix */ - nil, /* prefixChange */ + nil, /* syntheticPrefix */ writable, t.minFMV(), ) diff --git a/metamorphic/parser.go b/metamorphic/parser.go index da76c70729..42fb1f26c1 100644 --- a/metamorphic/parser.go +++ b/metamorphic/parser.go @@ -15,7 +15,6 @@ import ( "github.com/cockroachdb/errors" "github.com/cockroachdb/pebble" - "github.com/cockroachdb/pebble/sstable" ) type methodInfo struct { @@ -468,7 +467,7 @@ func (p *parser) parseCheckpointSpans(list []listElem) []pebble.CheckpointSpan { } func (p *parser) parseExternalObjsWithBounds(list []listElem) []externalObjWithBounds { - const numArgs = 6 + const numArgs = 5 if len(list)%numArgs != 0 { panic(p.errorf(list[0].pos, "expected number of arguments to be multiple of %d", numArgs)) } @@ -479,7 +478,6 @@ func (p *parser) parseExternalObjsWithBounds(list []listElem) []externalObjWithB list[2].expectToken(p, token.STRING) list[3].expectToken(p, token.STRING) list[4].expectToken(p, token.STRING) - list[5].expectToken(p, token.STRING) objs[i] = externalObjWithBounds{ externalObjID: p.parseObjID(list[0].pos, list[0].lit), bounds: pebble.KeyRange{ @@ -491,19 +489,15 @@ func (p *parser) parseExternalObjsWithBounds(list []listElem) []externalObjWithB objs[i].syntheticSuffix = syntheticSuffix } - contentPrefix := unquoteBytes(list[4].lit) - syntheticPrefix := unquoteBytes(list[5].lit) - if len(syntheticPrefix) > 0 || len(contentPrefix) > 0 { + syntheticPrefix := unquoteBytes(list[4].lit) + if len(syntheticPrefix) > 0 { if !bytes.HasPrefix(objs[i].bounds.Start, syntheticPrefix) { panic(fmt.Sprintf("invalid synthetic prefix %q %q", objs[i].bounds.Start, syntheticPrefix)) } if !bytes.HasPrefix(objs[i].bounds.End, syntheticPrefix) { panic(fmt.Sprintf("invalid synthetic prefix %q %q", objs[i].bounds.End, syntheticPrefix)) } - objs[i].prefixChange = &sstable.PrefixReplacement{ - ContentPrefix: contentPrefix, - SyntheticPrefix: syntheticPrefix, - } + objs[i].syntheticPrefix = syntheticPrefix } list = list[numArgs:] } diff --git a/scan_internal.go b/scan_internal.go index 06f2d47780..d45f6d8fb4 100644 --- a/scan_internal.go +++ b/scan_internal.go @@ -473,9 +473,8 @@ func (d *DB) truncateExternalFile( Size: file.Size, SyntheticSuffix: slices.Clone(file.SyntheticSuffix), } - if pr := file.PrefixReplacement; pr != nil { - sst.ContentPrefix = slices.Clone(pr.ContentPrefix) - sst.SyntheticPrefix = slices.Clone(pr.SyntheticPrefix) + if file.SyntheticPrefix.IsSet() { + sst.SyntheticPrefix = slices.Clone(sst.SyntheticPrefix) } needsLowerTruncate := cmp(lower, file.Smallest.UserKey) > 0 diff --git a/sstable/block.go b/sstable/block.go index 746118a0a6..4b12775fce 100644 --- a/sstable/block.go +++ b/sstable/block.go @@ -734,7 +734,7 @@ func (i *blockIter) maybeReplaceSuffix(allowInPlace bool) { return } // If ikey is cached or may get cached, we must copy - // UserKey to a new buffer before prefix replacement. + // UserKey to a new buffer before suffix replacement. i.synthSuffixBuf = append(i.synthSuffixBuf[:0], i.ikey.UserKey[:prefixLen]...) i.synthSuffixBuf = append(i.synthSuffixBuf, i.transforms.SyntheticSuffix...) i.ikey.UserKey = i.synthSuffixBuf @@ -1680,7 +1680,7 @@ start: // Inlined version of i.maybeReplaceSuffix(false /* allowInPlace */) prefixLen := i.split(i.ikey.UserKey) // If ikey is cached or may get cached, we must de-reference - // UserKey before prefix replacement. + // UserKey before suffix replacement. i.synthSuffixBuf = append(i.synthSuffixBuf[:0], i.ikey.UserKey[:prefixLen]...) i.synthSuffixBuf = append(i.synthSuffixBuf, i.transforms.SyntheticSuffix...) i.ikey.UserKey = i.synthSuffixBuf @@ -1766,7 +1766,7 @@ start: // Inlined version of i.maybeReplaceSuffix(false /* allowInPlace */) prefixLen := i.split(i.ikey.UserKey) // If ikey is cached or may get cached, we must de-reference - // UserKey before prefix replacement. + // UserKey before suffix replacement. i.synthSuffixBuf = append(i.synthSuffixBuf[:0], i.ikey.UserKey[:prefixLen]...) i.synthSuffixBuf = append(i.synthSuffixBuf, i.transforms.SyntheticSuffix...) i.ikey.UserKey = i.synthSuffixBuf diff --git a/sstable/prefix_replacing_iterator.go b/sstable/prefix_replacing_iterator.go deleted file mode 100644 index b3e93a3c7b..0000000000 --- a/sstable/prefix_replacing_iterator.go +++ /dev/null @@ -1,368 +0,0 @@ -// Copyright 2023 The LevelDB-Go and Pebble Authors. All rights reserved. Use -// of this source code is governed by a BSD-style license that can be found in -// the LICENSE file. - -package sstable - -import ( - "bytes" - "context" - "encoding/hex" - "fmt" - "slices" - - "github.com/cockroachdb/errors" - "github.com/cockroachdb/pebble/internal/base" - "github.com/cockroachdb/pebble/internal/invariants" - "github.com/cockroachdb/pebble/internal/keyspan" -) - -type prefixReplacingIterator struct { - i Iterator - cmp base.Compare - contentPrefix []byte - syntheticPrefix []byte - - // keyInRange is a valid key in the logical range that has the syntheticPrefix. - // When an argument to a seek function does not have the syntheticPrefix, - // keyInRange is used to determine if the argument key is before or after the - // range of keys produced by the iterator. - keyInRange []byte - - // arg and arg2 are buffers that are used to avoid allocations when rewriting - // keys that are provided as arguments. They always start with contentPrefix. - arg, arg2 []byte - - // res is used to avoid allocations when rewriting result keys. It always - // starts with syntheticPrefix. - res InternalKey - err error - // empty is set after a seek operation that returns no keys. - empty bool -} - -func errInputPrefixMismatch() error { - return errors.AssertionFailedf("key argument does not have prefix required for replacement") -} - -func errOutputPrefixMismatch() error { - return errors.AssertionFailedf("key returned does not have prefix required for replacement") -} - -var _ Iterator = (*prefixReplacingIterator)(nil) - -// newPrefixReplacingIterator wraps an iterator over keys that have -// `contentPrefix` in an iterator that will make them appear to have -// `syntheticPrefix`. Every key produced by the underlying iterator must have -// `contentPrefix`. -// -// keyInRange is a valid key that starts with syntheticPrefix. When a seek -// function is called with a key that does not start with syntheticPrefix, -// keyInRange is used to determine if the key is before or after the synthetic -// prefix range. -// -// INVARIANT: len(syntheticPrefix) > 0 && keyInRange stars with syntheticPrefix. -func newPrefixReplacingIterator( - i Iterator, contentPrefix, syntheticPrefix []byte, keyInRange []byte, cmp base.Compare, -) Iterator { - if invariants.Enabled { - if len(syntheticPrefix) == 0 { - panic("newPrefixReplacingIterator called without synthetic prefix") - } - if !bytes.HasPrefix(keyInRange, syntheticPrefix) { - panic(fmt.Sprintf("keyInRange %q does not have synthetic prefix %q", keyInRange, syntheticPrefix)) - } - } - return &prefixReplacingIterator{ - i: i, - cmp: cmp, - contentPrefix: contentPrefix, - syntheticPrefix: syntheticPrefix, - keyInRange: keyInRange, - arg: slices.Clone(contentPrefix), - arg2: slices.Clone(contentPrefix), - res: InternalKey{UserKey: slices.Clone(syntheticPrefix)}, - } -} - -func (p *prefixReplacingIterator) SetContext(ctx context.Context) { - p.i.SetContext(ctx) -} - -func (p *prefixReplacingIterator) rewriteArg(key []byte) []byte { - p.arg = append(p.arg[:len(p.contentPrefix)], key[len(p.syntheticPrefix):]...) - return p.arg -} - -func (p *prefixReplacingIterator) rewriteArg2(key []byte) []byte { - p.arg2 = append(p.arg2[:len(p.contentPrefix)], key[len(p.syntheticPrefix):]...) - return p.arg2 -} - -func (p *prefixReplacingIterator) rewriteResult( - k *InternalKey, v base.LazyValue, -) (*InternalKey, base.LazyValue) { - if k == nil { - return k, v - } - if !bytes.HasPrefix(k.UserKey, p.contentPrefix) { - p.err = errOutputPrefixMismatch() - if invariants.Enabled { - panic(p.err) - } - return nil, base.LazyValue{} - } - p.res.Trailer = k.Trailer - p.res.UserKey = append(p.res.UserKey[:len(p.syntheticPrefix)], k.UserKey[len(p.contentPrefix):]...) - return &p.res, v -} - -// SeekGE implements the Iterator interface. -func (p *prefixReplacingIterator) SeekGE( - key []byte, flags base.SeekGEFlags, -) (*InternalKey, base.LazyValue) { - p.empty = false - if !bytes.HasPrefix(key, p.syntheticPrefix) { - if p.cmp(key, p.keyInRange) > 0 { - p.empty = true - return nil, base.LazyValue{} - } - // Key must be before the range; use First instead. - return p.rewriteResult(p.i.First()) - } - return p.rewriteResult(p.i.SeekGE(p.rewriteArg(key), flags)) -} - -// SeekPrefixGE implements the Iterator interface. -func (p *prefixReplacingIterator) SeekPrefixGE( - prefix, key []byte, flags base.SeekGEFlags, -) (*InternalKey, base.LazyValue) { - p.empty = false - if invariants.Enabled && !bytes.HasPrefix(key, prefix) { - panic(fmt.Sprintf("key %q does not have prefix %q", key, prefix)) - } - if !bytes.HasPrefix(prefix, p.syntheticPrefix) { - // We never produce keys with this prefix; we can return nil. - p.empty = true - return nil, base.LazyValue{} - } - return p.rewriteResult(p.i.SeekPrefixGE(p.rewriteArg2(prefix), p.rewriteArg(key), flags)) -} - -// SeekLT implements the Iterator interface. -func (p *prefixReplacingIterator) SeekLT( - key []byte, flags base.SeekLTFlags, -) (*InternalKey, base.LazyValue) { - p.empty = false - if !bytes.HasPrefix(key, p.syntheticPrefix) { - if p.cmp(key, p.keyInRange) < 0 { - p.empty = true - return nil, base.LazyValue{} - } - // Key must be after the range. Use Last instead. - return p.rewriteResult(p.i.Last()) - } - return p.rewriteResult(p.i.SeekLT(p.rewriteArg(key), flags)) -} - -// First implements the Iterator interface. -func (p *prefixReplacingIterator) First() (*InternalKey, base.LazyValue) { - p.empty = false - return p.rewriteResult(p.i.First()) -} - -// Last implements the Iterator interface. -func (p *prefixReplacingIterator) Last() (*InternalKey, base.LazyValue) { - p.empty = false - return p.rewriteResult(p.i.Last()) -} - -// Next implements the Iterator interface. -func (p *prefixReplacingIterator) Next() (*InternalKey, base.LazyValue) { - if p.empty { - return nil, base.LazyValue{} - } - return p.rewriteResult(p.i.Next()) -} - -// NextPrefix implements the Iterator interface. -func (p *prefixReplacingIterator) NextPrefix(succKey []byte) (*InternalKey, base.LazyValue) { - if p.empty { - return nil, base.LazyValue{} - } - return p.rewriteResult(p.i.NextPrefix(p.rewriteArg(succKey))) -} - -// Prev implements the Iterator interface. -func (p *prefixReplacingIterator) Prev() (*InternalKey, base.LazyValue) { - if p.empty { - return nil, base.LazyValue{} - } - return p.rewriteResult(p.i.Prev()) -} - -// Error implements the Iterator interface. -func (p *prefixReplacingIterator) Error() error { - if p.err != nil { - return p.err - } - return p.i.Error() -} - -// Close implements the Iterator interface. -func (p *prefixReplacingIterator) Close() error { - return p.i.Close() -} - -// SetBounds implements the Iterator interface. -func (p *prefixReplacingIterator) SetBounds(lower, upper []byte) { - // Check if the underlying iterator requires un-rewritten bounds, i.e. if it - // is going to rewrite them itself or pass them to something e.g. vState that - // will rewrite them. - if x, ok := p.i.(interface{ SetBoundsWithSyntheticPrefix() bool }); ok && x.SetBoundsWithSyntheticPrefix() { - p.i.SetBounds(lower, upper) - return - } - if lower != nil { - lower = append([]byte{}, p.rewriteArg(lower)...) - } - if upper != nil { - upper = append([]byte{}, p.rewriteArg(upper)...) - } - p.i.SetBounds(lower, upper) -} - -func (p *prefixReplacingIterator) MaybeFilteredKeys() bool { - return p.i.MaybeFilteredKeys() -} - -// String implements the Iterator interface. -func (p *prefixReplacingIterator) String() string { - return fmt.Sprintf("%s [%s->%s]", p.i.String(), hex.EncodeToString(p.contentPrefix), hex.EncodeToString(p.syntheticPrefix)) -} - -func (p *prefixReplacingIterator) SetCloseHook(fn func(i Iterator) error) { - p.i.SetCloseHook(fn) -} - -type prefixReplacingFragmentIterator struct { - i keyspan.FragmentIterator - cmp base.Compare - - contentPrefix []byte - syntheticPrefix []byte - - // keyInRange is a valid key in the logical range that has the syntheticPrefix. - // When an argument to a seek function does not have the syntheticPrefix, - // keyInRange is used to determine if the argument key is before or after the - // range of keys produced by the iterator. - keyInRange []byte - - arg []byte - out1, out2 []byte -} - -// newPrefixReplacingFragmentIterator wraps a FragmentIterator over some reader -// that contains range keys in some key span to make those range keys appear to -// be remapped into some other key-span. -func newPrefixReplacingFragmentIterator( - i keyspan.FragmentIterator, - contentPrefix, syntheticPrefix []byte, - keyInRange []byte, - cmp base.Compare, -) keyspan.FragmentIterator { - return &prefixReplacingFragmentIterator{ - i: i, - cmp: cmp, - contentPrefix: contentPrefix, - syntheticPrefix: syntheticPrefix, - keyInRange: keyInRange, - arg: slices.Clone(contentPrefix), - out1: slices.Clone(syntheticPrefix), - out2: slices.Clone(syntheticPrefix), - } -} - -func (p *prefixReplacingFragmentIterator) rewriteArg(key []byte) ([]byte, error) { - if !bytes.HasPrefix(key, p.syntheticPrefix) { - return nil, errInputPrefixMismatch() - } - p.arg = append(p.arg[:len(p.contentPrefix)], key[len(p.syntheticPrefix):]...) - return p.arg, nil -} - -func (p *prefixReplacingFragmentIterator) rewriteSpan( - sp *keyspan.Span, err error, -) (*keyspan.Span, error) { - if sp == nil { - return sp, err - } - if !bytes.HasPrefix(sp.Start, p.contentPrefix) || !bytes.HasPrefix(sp.End, p.contentPrefix) { - return nil, errOutputPrefixMismatch() - } - sp.Start = append(p.out1[:len(p.syntheticPrefix)], sp.Start[len(p.contentPrefix):]...) - sp.End = append(p.out2[:len(p.syntheticPrefix)], sp.End[len(p.contentPrefix):]...) - return sp, nil -} - -// SeekGE implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) SeekGE(key []byte) (*keyspan.Span, error) { - if !bytes.HasPrefix(key, p.syntheticPrefix) { - if p.cmp(key, p.keyInRange) > 0 { - return nil, nil - } - // Key must be before the range; use First instead. - return p.First() - } - rewrittenKey, err := p.rewriteArg(key) - if err != nil { - return nil, err - } - return p.rewriteSpan(p.i.SeekGE(rewrittenKey)) -} - -// SeekLT implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) SeekLT(key []byte) (*keyspan.Span, error) { - if !bytes.HasPrefix(key, p.syntheticPrefix) { - if p.cmp(key, p.keyInRange) < 0 { - return nil, nil - } - // Key must be after the range; use Last instead. - return p.Last() - } - rewrittenKey, err := p.rewriteArg(key) - if err != nil { - return nil, err - } - return p.rewriteSpan(p.i.SeekLT(rewrittenKey)) -} - -// First implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) First() (*keyspan.Span, error) { - return p.rewriteSpan(p.i.First()) -} - -// Last implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) Last() (*keyspan.Span, error) { - return p.rewriteSpan(p.i.Last()) -} - -// Close implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) Next() (*keyspan.Span, error) { - return p.rewriteSpan(p.i.Next()) -} - -// Prev implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) Prev() (*keyspan.Span, error) { - return p.rewriteSpan(p.i.Prev()) -} - -// Close implements the FragmentIterator interface. -func (p *prefixReplacingFragmentIterator) Close() error { - return p.i.Close() -} - -// WrapChildren implements FragmentIterator. -func (p *prefixReplacingFragmentIterator) WrapChildren(wrap keyspan.WrapFn) { - p.i = wrap(p.i) -} diff --git a/sstable/prefix_replacing_iterator_test.go b/sstable/prefix_replacing_iterator_test.go deleted file mode 100644 index e37f420a1b..0000000000 --- a/sstable/prefix_replacing_iterator_test.go +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2023 The LevelDB-Go and Pebble Authors. All rights reserved. Use -// of this source code is governed by a BSD-style license that can be found in -// the LICENSE file. - -package sstable - -import ( - "context" - "encoding/binary" - "fmt" - "testing" - - "github.com/cockroachdb/pebble/internal/base" - "github.com/stretchr/testify/require" -) - -func TestPrefixReplacingIterator(t *testing.T) { - for _, tc := range []struct{ from, to []byte }{ - {from: []byte(""), to: []byte("bb")}, - {from: []byte("aa"), to: []byte("aa")}, - {from: []byte("aa"), to: []byte("bb")}, - {from: []byte("bb"), to: []byte("aa")}, - {from: []byte("aa"), to: []byte("zzz")}, - {from: []byte("zzz"), to: []byte("aa")}, - } { - t.Run(fmt.Sprintf("%s_%s", tc.from, tc.to), func(t *testing.T) { - r := buildTestTable(t, 20, 256, 256, DefaultCompression, tc.from) - defer r.Close() - rawIter, err := r.NewIter(NoTransforms, nil, nil) - require.NoError(t, err) - defer rawIter.Close() - - raw := rawIter.(*singleLevelIterator) - - it := newPrefixReplacingIterator(raw, tc.from, tc.to, tc.to, DefaultComparer.Compare) - - kMin := []byte{0} - kMax := []byte("~") - k := func(i uint64) []byte { - return binary.BigEndian.AppendUint64(tc.to[:len(tc.to):len(tc.to)], i) - } - - var got *base.InternalKey - - t.Run("FirstNextLast", func(t *testing.T) { - got, _ = it.First() - require.Equal(t, k(0), got.UserKey) - got, _ = it.Next() - require.Equal(t, k(1), got.UserKey) - got, _ = it.Next() - require.Equal(t, k(2), got.UserKey) - got, _ = it.Prev() - require.Equal(t, k(1), got.UserKey) - got, _ = it.Last() - require.Equal(t, k(19), got.UserKey) - }) - - t.Run("SetBounds", func(t *testing.T) { - it.SetBounds(k(5), k(15)) - defer it.SetBounds(nil, nil) - - got, _ = it.SeekGE(k(16), base.SeekGEFlagsNone) - require.Nil(t, got) - - got, _ = it.SeekGE(k(14), base.SeekGEFlagsNone) - require.Equal(t, k(14), got.UserKey) - - got, _ = it.SeekLT(k(100), base.SeekLTFlagsNone) - require.Equal(t, k(19), got.UserKey) - }) - - t.Run("SetHookAndCtx", func(t *testing.T) { - it.SetCloseHook(func(i Iterator) error { return nil }) - require.NotNil(t, raw.closeHook) - it.SetCloseHook(nil) - require.Nil(t, raw.closeHook) - - require.Equal(t, context.Background(), raw.ctx) - ctx := context.WithValue(context.Background(), struct{}{}, "") - it.SetContext(ctx) - require.Equal(t, ctx, raw.ctx) - it.SetContext(context.Background()) - require.Equal(t, context.Background(), raw.ctx) - }) - - t.Run("SeekGE", func(t *testing.T) { - got, _ = it.SeekGE(kMin, base.SeekGEFlagsNone) - require.Equal(t, k(0), got.UserKey) - - got, _ = it.SeekGE(k(0), base.SeekGEFlagsNone) - require.Equal(t, k(0), got.UserKey) - - got, _ = it.SeekGE(k(10), base.SeekGEFlagsNone) - require.Equal(t, k(10), got.UserKey) - - got, _ = it.SeekGE(k(100), base.SeekGEFlagsNone) - require.Nil(t, got) - - got, _ = it.SeekGE(kMax, base.SeekGEFlagsNone) - require.Nil(t, got) - }) - - t.Run("SeekPrefixGE", func(t *testing.T) { - got, _ = it.SeekPrefixGE(tc.to, k(0), base.SeekGEFlagsNone) - require.Equal(t, k(0), got.UserKey) - - got, _ = it.SeekPrefixGE(tc.to, k(10), base.SeekGEFlagsNone) - require.Equal(t, k(10), got.UserKey) - - got, _ = it.Next() - require.Equal(t, k(11), got.UserKey) - - got, _ = it.SeekPrefixGE(tc.to, k(100), base.SeekGEFlagsNone) - require.Nil(t, got) - }) - - t.Run("SeekLT", func(t *testing.T) { - got, _ = it.SeekLT(kMin, base.SeekLTFlagsNone) - require.Nil(t, got) - - got, _ = it.SeekLT(k(0), base.SeekLTFlagsNone) - require.Nil(t, got) - - got, _ = it.SeekLT(k(10), base.SeekLTFlagsNone) - require.Equal(t, k(9), got.UserKey) - - got, _ = it.Next() - require.Equal(t, k(10), got.UserKey) - - got, _ = it.SeekLT(k(100), base.SeekLTFlagsNone) - require.Equal(t, k(19), got.UserKey) - - got, _ = it.SeekLT(kMax, base.SeekLTFlagsNone) - require.Equal(t, k(19), got.UserKey) - }) - }) - } -} diff --git a/sstable/reader.go b/sstable/reader.go index 0eadf38d2c..dbec118fc7 100644 --- a/sstable/reader.go +++ b/sstable/reader.go @@ -9,6 +9,7 @@ import ( "cmp" "context" "encoding/binary" + "fmt" "io" "os" "slices" @@ -189,6 +190,23 @@ func (sp SyntheticPrefix) IsSet() bool { return len(sp) > 0 } +// Apply prepends the synthetic prefix to a key. +func (sp SyntheticPrefix) Apply(key []byte) []byte { + res := make([]byte, 0, len(sp)+len(key)) + res = append(res, sp...) + res = append(res, key...) + return res +} + +// Invert removes the synthetic prefix from a key. +func (sp SyntheticPrefix) Invert(key []byte) []byte { + res, ok := bytes.CutPrefix(key, sp) + if !ok { + panic(fmt.Sprintf("unexpected prefix: %s", key)) + } + return res +} + // rawTombstonesOpt is a Reader open option for specifying that range // tombstones returned by Reader.NewRangeDelIter() should not be // fragmented. Used by debug tools to get a raw view of the tombstones diff --git a/sstable/reader_common.go b/sstable/reader_common.go index 9a866a6f5e..3e80297904 100644 --- a/sstable/reader_common.go +++ b/sstable/reader_common.go @@ -5,9 +5,7 @@ package sstable import ( - "bytes" "context" - "fmt" "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/keyspan" @@ -70,38 +68,3 @@ type SyntheticSeqNum uint64 // NoSyntheticSeqNum is the default zero value for SyntheticSeqNum, which // disables overriding the sequence number. const NoSyntheticSeqNum SyntheticSeqNum = 0 - -// PrefixReplacement represents a read-time replacement of a key prefix. -type PrefixReplacement struct { - // ContentPrefix is the existing prefix that each key is expected to have. - ContentPrefix []byte - // SyntheticPrefix replaces the ContentPrefix in all keys. If ContentPrefix is - // empty, we are just prepending the synthetic prefix. - SyntheticPrefix []byte -} - -// UsePrefixReplacementIterator returns true if the prefix replacement iterator -// wrapper should be used. -func (p *PrefixReplacement) UsePrefixReplacementIterator() bool { - return p != nil && len(p.ContentPrefix) > 0 -} - -// Apply replaces the content prefix in the key with the synthetic prefix. -func (p *PrefixReplacement) Apply(key []byte) []byte { - return p.replace(key, p.ContentPrefix, p.SyntheticPrefix) -} - -// Invert replaces the synthetic prefix in the key with the content prefix. -func (p *PrefixReplacement) Invert(src []byte) []byte { - return p.replace(src, p.SyntheticPrefix, p.ContentPrefix) -} - -func (p *PrefixReplacement) replace(key, from, to []byte) []byte { - if !bytes.HasPrefix(key, from) { - panic(fmt.Sprintf("unexpected prefix in replace: %s", key)) - } - result := make([]byte, 0, len(to)+(len(key)-len(from))) - result = append(result, to...) - result = append(result, key[len(from):]...) - return result -} diff --git a/sstable/reader_iter_single_lvl.go b/sstable/reader_iter_single_lvl.go index b5292049a6..bf119ac92f 100644 --- a/sstable/reader_iter_single_lvl.go +++ b/sstable/reader_iter_single_lvl.go @@ -258,20 +258,8 @@ func (i *singleLevelIterator) maybeVerifyKey( if invariants.Enabled && iKey != nil && i.vState != nil { key := iKey.UserKey v := i.vState - var uc, lc int - if p := v.prefixChange; p != nil && len(p.ContentPrefix) > 0 { - if !bytes.HasPrefix(key, p.ContentPrefix) { - panic(fmt.Sprintf("key %q does not have content prefix %q", key, v.prefixChange.ContentPrefix)) - } - // We are assuming that the key comparator works if we just skip the - // prefix portion that we are replacing. This is true for all known - // implementations. - lc = i.cmp(key[len(p.ContentPrefix):], v.lower.UserKey[len(p.SyntheticPrefix):]) - uc = i.cmp(key[len(p.ContentPrefix):], v.upper.UserKey[len(p.SyntheticPrefix):]) - } else { - lc = i.cmp(key, v.lower.UserKey) - uc = i.cmp(key, v.upper.UserKey) - } + lc := i.cmp(key, v.lower.UserKey) + uc := i.cmp(key, v.upper.UserKey) if lc < 0 || uc > 0 || (uc == 0 && v.upper.IsExclusiveSentinel()) { panic(fmt.Sprintf("key %q out of singleLeveliterator virtual bounds %s %s", key, v.lower.UserKey, v.upper.UserKey)) } diff --git a/sstable/reader_virtual.go b/sstable/reader_virtual.go index e77d66217b..0fe4c3eb08 100644 --- a/sstable/reader_virtual.go +++ b/sstable/reader_virtual.go @@ -34,7 +34,6 @@ type virtualState struct { fileNum base.FileNum Compare Compare isSharedIngested bool - prefixChange *PrefixReplacement } // VirtualReaderParams are the parameters necessary to create a VirtualReader. @@ -48,8 +47,6 @@ type VirtualReaderParams struct { // BackingSize is the total size of the backing table. The ratio between Size // and BackingSize is used to estimate statistics. BackingSize uint64 - // TODO(radu): these should be moved to sstable.IterTransforms. - PrefixReplacement *PrefixReplacement } // MakeVirtualReader is used to contruct a reader which can read from virtual @@ -61,7 +58,6 @@ func MakeVirtualReader(reader *Reader, p VirtualReaderParams) VirtualReader { fileNum: p.FileNum, Compare: reader.Compare, isSharedIngested: p.IsSharedIngested, - prefixChange: p.PrefixReplacement, } v := VirtualReader{ vState: vState, @@ -101,15 +97,8 @@ func (v *VirtualReader) NewCompactionIter( rp ReaderProvider, bufferPool *BufferPool, ) (Iterator, error) { - i, err := v.reader.newCompactionIter( + return v.reader.newCompactionIter( transforms, bytesIterated, categoryAndQoS, statsCollector, rp, &v.vState, bufferPool) - if err == nil && v.vState.prefixChange.UsePrefixReplacementIterator() { - i = newPrefixReplacingIterator( - i, v.vState.prefixChange.ContentPrefix, v.vState.prefixChange.SyntheticPrefix, - v.vState.lower.UserKey, v.reader.Compare, - ) - } - return i, err } // NewIterWithBlockPropertyFiltersAndContextEtc wraps @@ -127,17 +116,9 @@ func (v *VirtualReader) NewIterWithBlockPropertyFiltersAndContextEtc( statsCollector *CategoryStatsCollector, rp ReaderProvider, ) (Iterator, error) { - i, err := v.reader.newIterWithBlockPropertyFiltersAndContext( + return v.reader.newIterWithBlockPropertyFiltersAndContext( ctx, transforms, lower, upper, filterer, useFilterBlock, stats, categoryAndQoS, statsCollector, rp, &v.vState) - // NB: for block level prefix replacement, - if err == nil && v.vState.prefixChange.UsePrefixReplacementIterator() { - i = newPrefixReplacingIterator( - i, v.vState.prefixChange.ContentPrefix, v.vState.prefixChange.SyntheticPrefix, - v.vState.lower.UserKey, v.reader.Compare, - ) - } - return i, err } // ValidateBlockChecksumsOnBacking will call ValidateBlockChecksumsOnBacking on the underlying reader. @@ -160,20 +141,6 @@ func (v *VirtualReader) NewRawRangeDelIter( lower := &v.vState.lower upper := &v.vState.upper - if v.vState.prefixChange.UsePrefixReplacementIterator() { - lower = &InternalKey{UserKey: v.vState.prefixChange.Invert(lower.UserKey), Trailer: lower.Trailer} - upper = &InternalKey{UserKey: v.vState.prefixChange.Invert(upper.UserKey), Trailer: upper.Trailer} - - iter = keyspan.Truncate( - v.reader.Compare, iter, lower.UserKey, upper.UserKey, - lower, upper, !v.vState.upper.IsExclusiveSentinel(), /* panicOnUpperTruncate */ - ) - return newPrefixReplacingFragmentIterator( - iter, v.vState.prefixChange.ContentPrefix, v.vState.prefixChange.SyntheticPrefix, - v.vState.lower.UserKey, v.reader.Compare, - ), nil - } - // Truncation of spans isn't allowed at a user key that also contains points // in the same virtual sstable, as it would lead to covered points getting // uncovered. Set panicOnUpperTruncate to true if the file's upper bound @@ -229,19 +196,6 @@ func (v *VirtualReader) NewRawRangeKeyIter( iter = transformIter } - if v.vState.prefixChange.UsePrefixReplacementIterator() { - lower = &InternalKey{UserKey: v.vState.prefixChange.Invert(lower.UserKey), Trailer: lower.Trailer} - upper = &InternalKey{UserKey: v.vState.prefixChange.Invert(upper.UserKey), Trailer: upper.Trailer} - iter = keyspan.Truncate( - v.reader.Compare, iter, lower.UserKey, upper.UserKey, - lower, upper, !v.vState.upper.IsExclusiveSentinel(), /* panicOnUpperTruncate */ - ) - return newPrefixReplacingFragmentIterator( - iter, v.vState.prefixChange.ContentPrefix, v.vState.prefixChange.SyntheticPrefix, - v.vState.lower.UserKey, v.reader.Compare, - ), nil - } - // Truncation of spans isn't allowed at a user key that also contains points // in the same virtual sstable, as it would lead to covered points getting // uncovered. Set panicOnUpperTruncate to true if the file's upper bound @@ -287,10 +241,6 @@ func (v *virtualState) constrainBounds( last = end } } - if v.prefixChange.UsePrefixReplacementIterator() { - first = v.prefixChange.Invert(first) - last = v.prefixChange.Invert(last) - } // TODO(bananabrick): What if someone passes in bounds completely outside of // virtual sstable bounds? return lastKeyInclusive, first, last diff --git a/testdata/ingest_external b/testdata/ingest_external index ef551b7d56..1691084514 100644 --- a/testdata/ingest_external +++ b/testdata/ingest_external @@ -169,45 +169,8 @@ set bg foo set bh foo ---- -ingest-external -f6 bounds=(cf,ci) prefix-replace=(b,c) ----- - -iter -first -next -next -next -next -next -next -next -next -next -next -next -next -next -next ----- -a: (foo, .) -b: (bar, .) -c: (foobarbaz, .) -cf: (foo, .) -cg: (foo, .) -ch: (foo, .) -d: (haha, .) -e: (something, .) -f: (foo, .) -ff: (foo, .) -fg: (foo, .) -fh: (foo, .) -g: (foo, .) -h: (foo, .) -. - -# Test that ingestion with a prefix or suffix replacement rule fails on older -# major versions +# Test that ingestion with a syntehtic prefix or suffix fails on older +# major versions. reset format-major-version=16 ---- @@ -218,18 +181,11 @@ set eg foo set eh foo ---- -ingest-external -f5 bounds=(ff,fi) prefix-replace=(e,f) ----- -pebble: format major version too old for synthetic prefix ingestion - - ingest-external f5 bounds=(ff,fi) synthetic-prefix=(f) ---- pebble: format major version too old for synthetic prefix ingestion - ingest-external f5 bounds=(ff,fi) synthetic-suffix=@5 ---- @@ -434,71 +390,6 @@ seek-lt de ---- ci: (foo, .) -reset ----- - - -# Test compactions with synthetic prefix replacement. -reset ----- - -build-remote ext -set ax ax -set da da -set db db -set dc dc -set ux ux ----- - -# Replace prefix with one greater than the original one. -ingest-external -ext bounds=(ea,ed) prefix-replace=(d,e) ----- - -# Replace prefix with one smaller than the original one. -ingest-external -ext bounds=(ba,bd) prefix-replace=(d,b) ----- - -# Write some keys so we actually perform a compaction. -batch -set a a -set c c -set f f ----- - -compact a z ----- - -lsm ----- -L6: - 000008:[a#0,SET-f#0,SET] - -# Make sure we see both ba..bc and ea..ec. -iter -first -next -next -next -next -next -next -next -next -next ----- -a: (a, .) -ba: (da, .) -bb: (db, .) -bc: (dc, .) -c: (c, .) -ea: (da, .) -eb: (db, .) -ec: (dc, .) -f: (f, .) -. - # Test compactions with prefix synthesis. reset ----