diff --git a/metamorphic/key_manager.go b/metamorphic/key_manager.go index 5cdf944f1f..c4fe6903f6 100644 --- a/metamorphic/key_manager.go +++ b/metamorphic/key_manager.go @@ -25,6 +25,14 @@ type keyMeta struct { history keyHistory } +// all returns true if all the operations in the history are of the provided op +// types. +func (m *keyMeta) all(typs ...OpType) bool { + return slices.ContainsFunc(m.history, func(i keyHistoryItem) bool { + return !slices.Contains(typs, i.opType) + }) +} + func (m *keyMeta) clear() { m.history = m.history[:0] } @@ -170,14 +178,9 @@ func (b *bounds) Expand(cmp base.Compare, other bounds) { // infallible, because a runtime failure would cause the key manager's state to // diverge from the runtime object state. Ingestion operations pose an obstacle, // because the generator may generate ingestions that fail due to overlapping -// sstables. Today, this complication is sidestepped by avoiding ingestion of -// multiple batches containing deletes or single deletes since loss of those -// specific operations on a key are what we cannot tolerate (doing SingleDelete -// on a key that has not been written to because the Set was lost is harmless). -// -// TODO(jackson): Instead, compute smallest and largest bounds of batches so -// that we know at generation-time whether or not an ingestion operation will -// fail and can avoid updating key state. +// sstables. The key manager tracks the bounds of keys written to each object, +// and uses this to infer at generation time which ingestions will fail, +// maintaining key histories accordingly. type keyManager struct { kf KeyFormat @@ -307,9 +310,13 @@ func (k *keyManager) KeysForExternalIngest(obj externalObjWithBounds) []keyMeta n := k.kf.Comparer.Split(km.key) km.key = append(km.key[:n:n], obj.syntheticSuffix...) } - if lastPrefix != nil && k.kf.Comparer.Equal(lastPrefix, km.key[:k.kf.Comparer.Split(km.key)]) { - // We only keep the first of every unique prefix for external ingests. - // See the use of uniquePrefixes in newExternalObjOp. + // We only keep the first of every unique prefix for external ingests. + // See the use of uniquePrefixes in newExternalObjOp. However, we must + // ignore keys that only exist as a discretized range deletion for this + // purpose. + // TODO(jackson): Track range deletions separately. + if !km.all(OpWriterDeleteRange) && lastPrefix != nil && + k.kf.Comparer.Equal(lastPrefix, km.key[:k.kf.Comparer.Split(km.key)]) { continue } lastPrefix = append(lastPrefix[:0], km.key[:k.kf.Comparer.Split(km.key)]...) diff --git a/metamorphic/ops.go b/metamorphic/ops.go index 9b133a2ac8..bb2bafad82 100644 --- a/metamorphic/ops.go +++ b/metamorphic/ops.go @@ -1922,9 +1922,9 @@ func (o *dbRatchetFormatMajorVersionOp) run(t *Test, h historyRecorder) { // versions, making the presence of an error and the error message itself // non-deterministic if we attempt to upgrade to an older version. // - //Regardless, subsequent operations should behave identically, which is what - //we're really aiming to test by including this format major version ratchet - //operation. + // Regardless, subsequent operations should behave identically, which is + // what we're really aiming to test by including this format major version + // ratchet operation. if t.getDB(o.dbID).FormatMajorVersion() < o.vers { err = t.getDB(o.dbID).RatchetFormatMajorVersion(o.vers) } diff --git a/metamorphic/testdata/key_manager b/metamorphic/testdata/key_manager index 3c0abda124..cff36b397e 100644 --- a/metamorphic/testdata/key_manager +++ b/metamorphic/testdata/key_manager @@ -475,3 +475,31 @@ conflicts merging batch1 into db1: "foo" conflicts merging batch1 (collapsed) into db1: (none) [db1.Merge("foo", "foo")] can singledel on db1: (none) + +# Regression test for #4267. In the below example, foo@3 must not be eligible +# for single delete on db1 at the end of the test. The external object will end +# up containing the point key foo@3 (NOT foo@5). + +reset +---- + +run +add-new-key foo@5 +op db1.Set("foo@5", "foo5") +add-new-key foo@3 +op db1.Set("foo@3", "foo3") +op batch1.DeleteRange("a", "foo@3") +op batch1.Set("foo@3", "foo3batch1") +op external1 = batch1.NewExternalObj() +op db1.IngestExternalFiles(external1, "a", "z", "", "") +singledel-keys db1 +---- +"foo@5" is new +[db1.Set("foo@5", "foo5")] +"foo@3" is new +[db1.Set("foo@3", "foo3")] +[batch1.DeleteRange("a", "foo@3")] +[batch1.Set("foo@3", "foo3batch1")] +[external1 = batch1.NewExternalObj()] +[db1.IngestExternalFiles(external1, "a" /* start */, "z" /* end */, "" /* syntheticSuffix */, "" /* syntheticPrefix */)] +can singledel on db1: "foo@5" \ No newline at end of file