db: consider range keys in delete-only compactions

Currently, when constructing delete-only compactions hints, only range
delete spans are considered. With the introduction of range keys, the
current hint construction logic is incorrect. Specifically, a hint
constructed from a range delete may completely cover a table containing
range keys. Currently, this table would be marked as eligible for
deletion. However, the table should only be deleted if the range keys
are also deleted.

Make use of the `tableRangedDeletionIter` for construction of
delete-only compaction hints. A new `deleteCompactionHintType` type is
used to distinguish between hints generated by a range delete (which may
not delete tables containing range keys), hints generated from a range
key delete (which may not delete tables containing point keys), and
hints generated from a span with both a range delete and range key
delete (which may delete any covering table).

compaction.go

@@ -1816,13 +1816,63 @@ func (d *DB) maybeScheduleCompactionPicker(
 	}
 }
 
+// deleteCompactionHintType indicates whether the deleteCompactionHint was
+// generated from a span containing a range del (point key only), a range key
+// delete (range key only), or both a point and range key.
+type deleteCompactionHintType uint8
+
+const (
+	// NOTE: While these are primarily used as enumeration types, they are also
+	// used for some bitwise operations. Care should be taken when updating.
+	deleteCompactionHintTypeUnknown deleteCompactionHintType = iota
+	deleteCompactionHintTypePointKeyOnly
+	deleteCompactionHintTypeRangeKeyOnly
+	deleteCompactionHintTypePointAndRangeKey
+)
+
+// String implements fmt.Stringer.
+func (h deleteCompactionHintType) String() string {
+	switch h {
+	case deleteCompactionHintTypeUnknown:
+		return "unknown"
+	case deleteCompactionHintTypePointKeyOnly:
+		return "point-key-only"
+	case deleteCompactionHintTypeRangeKeyOnly:
+		return "range-key-only"
+	case deleteCompactionHintTypePointAndRangeKey:
+		return "point-and-range-key"
+	default:
+		panic(fmt.Sprintf("unknown hint type: %d", h))
+	}
+}
+
+// compactionHintFromKeys returns a deleteCompactionHintType given a slice of
+// keyspan.Keys.
+func compactionHintFromKeys(keys []keyspan.Key) deleteCompactionHintType {
+	var hintType deleteCompactionHintType
+	for _, k := range keys {
+		switch k.Kind() {
+		case base.InternalKeyKindRangeDelete:
+			hintType |= deleteCompactionHintTypePointKeyOnly
+		case base.InternalKeyKindRangeKeyDelete:
+			hintType |= deleteCompactionHintTypeRangeKeyOnly
+		default:
+			panic(fmt.Sprintf("unsupported key kind: %s", k.Kind()))
+		}
+	}
+	return hintType
+}
+
 // A deleteCompactionHint records a user key and sequence number span that has been
 // deleted by a range tombstone. A hint is recorded if at least one sstable
 // falls completely within both the user key and sequence number spans.
 // Once the tombstones and the observed completely-contained sstables fall
 // into the same snapshot stripe, a delete-only compaction may delete any
 // sstables within the range.
 type deleteCompactionHint struct {
+	// The type of key span that generated this hint (point key, range key, or
+	// both).
+	hintType deleteCompactionHintType
 	// start and end are user keys specifying a key range [start, end) of
 	// deleted keys.
 	start []byte
@@ -1850,9 +1900,12 @@ type deleteCompactionHint struct {
 }
 
 func (h deleteCompactionHint) String() string {
-	return fmt.Sprintf("L%d.%s %s-%s seqnums(tombstone=%d-%d, file-smallest=%d)",
+	return fmt.Sprintf(
+		"L%d.%s %s-%s seqnums(tombstone=%d-%d, file-smallest=%d, type=%s)",
 		h.tombstoneLevel, h.tombstoneFile.FileNum, h.start, h.end,
-		h.tombstoneSmallestSeqNum, h.tombstoneLargestSeqNum, h.fileSmallestSeqNum)
+		h.tombstoneSmallestSeqNum, h.tombstoneLargestSeqNum, h.fileSmallestSeqNum,
+		h.hintType,
+	)
 }
 
 func (h *deleteCompactionHint) canDelete(cmp Compare, m *fileMetadata, snapshots []uint64) bool {
@@ -1873,6 +1926,27 @@ func (h *deleteCompactionHint) canDelete(cmp Compare, m *fileMetadata, snapshots
 		return false
 	}
 
+	switch h.hintType {
+	case deleteCompactionHintTypePointKeyOnly:
+		// A hint generated by a range del span cannot delete tables that contain
+		// range keys.
+		if m.HasRangeKeys {
+			return false
+		}
+	case deleteCompactionHintTypeRangeKeyOnly:
+		// A hint generated by a range key del span cannot delete tables that
+		// contain point keys.
+		if m.HasPointKeys {
+			return false
+		}
+	case deleteCompactionHintTypePointAndRangeKey:
+		// A hint from a span that contains both range dels *and* range keys can
+		// only be deleted if both bounds fall within the hint. The next check takes
+		// care of this.
+	default:
+		panic(fmt.Sprintf("pebble: unknown delete compaction hint type: %d", h.hintType))
+	}
+
 	// The file's keys must be completely contained within the hint range.
 	return cmp(h.start, m.Smallest.UserKey) <= 0 && cmp(m.Largest.UserKey, h.end) < 0
 }
@@ -1991,12 +2065,6 @@ func checkDeleteCompactionHints(
 				if m.Compacting || !h.canDelete(cmp, m, snapshots) || files[m] {
 					continue
 				}
-				if m.HasRangeKeys {
-					// TODO(bilal): Remove this conditional when deletion hints work well
-					// with sstables containing range keys.
-					continue
-				}
-
 				if files == nil {
 					// Construct files lazily, assuming most calls will not
 					// produce delete-only compactions.

compaction_test.go

@@ -1848,32 +1848,67 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 	}()
 
 	var compactInfo *CompactionInfo // protected by d.mu
+	reset := func() (*Options, error) {
+		if d != nil {
+			compactInfo = nil
+			if err := d.Close(); err != nil {
+				return nil, err
+			}
+		}
+		opts := &Options{
+			FS:         vfs.NewMem(),
+			DebugCheck: DebugCheckLevels,
+			EventListener: EventListener{
+				CompactionEnd: func(info CompactionInfo) {
+					if compactInfo != nil {
+						return
+					}
+					compactInfo = &info
+				},
+			},
+			FormatMajorVersion: FormatNewest,
+		}
 
+		// Collection of table stats can trigger compactions. As we want full
+		// control over when compactions are run, disable stats by default.
+		opts.private.disableTableStats = true
+
+		// Disable automatic compactions to prevent the range key-only tables from
+		// being compacted away after they are created. Compactions do not yet
+		// understand that these tables need to remain in the LSM.
+		// TODO(travers): Revisit this once compactions support range keys.
+		opts.DisableAutomaticCompactions = true
+
+		return opts, nil
+	}
+
+	compactionString := func() string {
+		for d.mu.compact.compactingCount > 0 {
+			d.mu.compact.cond.Wait()
+		}
+
+		s := "(none)"
+		if compactInfo != nil {
+			// Fix the job ID and durations for determinism.
+			compactInfo.JobID = 100
+			compactInfo.Duration = time.Second
+			compactInfo.TotalDuration = 2 * time.Second
+			s = compactInfo.String()
+			compactInfo = nil
+		}
+		return s
+	}
+
+	var err error
+	var opts *Options
 	datadriven.RunTest(t, "testdata/compaction_delete_only_hints",
 		func(td *datadriven.TestData) string {
 			switch td.Cmd {
 			case "define":
-				if d != nil {
-					compactInfo = nil
-					if err := d.Close(); err != nil {
-						return err.Error()
-					}
-				}
-				opts := &Options{
-					FS:         vfs.NewMem(),
-					DebugCheck: DebugCheckLevels,
-					EventListener: EventListener{
-						CompactionEnd: func(info CompactionInfo) {
-							compactInfo = &info
-						},
-					},
+				opts, err = reset()
+				if err != nil {
+					return err.Error()
 				}
-
-				// Collection of table stats can trigger compactions. As we want full
-				// control over when compactions are run, disable stats by default.
-				opts.private.disableTableStats = true
-
-				var err error
 				d, err = runDBDefineCmd(td, opts)
 				if err != nil {
 					return err.Error()
@@ -1902,7 +1937,20 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 						FileNum: base.FileNum(parseUint64(parts[1])),
 					}
 
+					var hintType deleteCompactionHintType
+					switch typ := parts[7]; typ {
+					case "point_key_only":
+						hintType = deleteCompactionHintTypePointKeyOnly
+					case "range_key_only":
+						hintType = deleteCompactionHintTypeRangeKeyOnly
+					case "point_and_range_key":
+						hintType = deleteCompactionHintTypePointAndRangeKey
+					default:
+						return fmt.Sprintf("unknown hint type: %s", typ)
+					}
+
 					h := deleteCompactionHint{
+						hintType:                hintType,
 						start:                   start,
 						end:                     end,
 						fileSmallestSeqNum:      parseUint64(parts[4]),
@@ -1954,9 +2002,6 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 			case "maybe-compact":
 				d.mu.Lock()
 				d.maybeScheduleCompaction()
-				for d.mu.compact.compactingCount > 0 {
-					d.mu.compact.cond.Wait()
-				}
 
 				var buf bytes.Buffer
 				fmt.Fprintf(&buf, "Deletion hints:\n")
@@ -1967,15 +2012,7 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 					fmt.Fprintf(&buf, "  (none)\n")
 				}
 				fmt.Fprintf(&buf, "Compactions:\n")
-				s := "(none)"
-				if compactInfo != nil {
-					// Fix the job ID and durations for determinism.
-					compactInfo.JobID = 100
-					compactInfo.Duration = time.Second
-					compactInfo.TotalDuration = 2 * time.Second
-					s = compactInfo.String()
-				}
-				fmt.Fprintf(&buf, "  %s", s)
+				fmt.Fprintf(&buf, "  %s", compactionString())
 				d.mu.Unlock()
 				return buf.String()
 
@@ -2009,32 +2046,12 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 					return err.Error()
 				}
 
-				compactionString := func() string {
-					for d.mu.compact.compactingCount > 0 {
-						d.mu.compact.cond.Wait()
-					}
-
-					s := "(none)"
-					if compactInfo != nil {
-						// Fix the job ID and durations for determinism.
-						compactInfo.JobID = 100
-						compactInfo.Duration = time.Second
-						compactInfo.TotalDuration = 2 * time.Second
-						s = compactInfo.String()
-						compactInfo = nil
-					}
-					return s
-				}
-
 				d.mu.Lock()
 				// Closing the snapshot may have triggered a compaction.
 				str := compactionString()
 				d.mu.Unlock()
 				return str
 
-			case "wait-pending-table-stats":
-				return runTableStatsCmd(td, d)
-
 			case "iter":
 				snap := Snapshot{
 					db:     d,
@@ -2043,6 +2060,36 @@ func TestCompactionDeleteOnlyHints(t *testing.T) {
 				iter := snap.NewIter(nil)
 				return runIterCmd(td, iter, true)
 
+			case "reset":
+				opts, err = reset()
+				if err != nil {
+					return err.Error()
+				}
+				d, err = Open("", opts)
+				if err != nil {
+					return err.Error()
+				}
+				return ""
+
+			case "ingest":
+				// Compactions / flushes do not yet fully support range keys. The
+				// "ingest" operation exists to allow tables containing range keys to be
+				// added to the LSM via ingest, rather than a flush.
+				// TODO(travers): Revisit this once compactions support range keys.
+				if err = runBuildCmd(td, d, d.opts.FS); err != nil {
+					return err.Error()
+				}
+				if err = runIngestCmd(td, d, d.opts.FS); err != nil {
+					return err.Error()
+				}
+				return "OK"
+
+			case "describe-lsm":
+				d.mu.Lock()
+				s := d.mu.versions.currentVersion().String()
+				d.mu.Unlock()
+				return s
+
 			default:
 				return fmt.Sprintf("unknown command: %s", td.Cmd)
 			}