manifest: unref files with range keys when version refcnt is zero

A manifest version uses reference counting to ensure that obsolete
SSTables are only removed once it is safe to do so (i.e. no iterators or
snapshots are open referencing tables in a older manifest version).

In cockroachdb#1771, a new slice of `LevelMetadata` was added to support range
keys. A slice already existed for point keys (plus range dels).
Currently, when a version's ref count reaches zero, only the files
present in the point key `LevelMetadata` slice have their ref counts
decremented. If a file also contains range keys, the reference count
never becomes zero and the file becomes "zombied", and can never be
deleted from disk.

Decrement the ref counts on files present in the range keys
`LevelMetadata` slice, allowing the ref counts to be correctly zeroed.

Add regression test to exercise the bug observed in
cockroachdb/cockroach#90149.

Fix cockroachdb/cockroach#90149.

compaction_test.go

@@ -2200,6 +2200,13 @@ func TestCompactionTombstones(t *testing.T) {
 				d.mu.Unlock()
 				return str
 
+			case "close":
+				if err := d.Close(); err != nil {
+					return err.Error()
+				}
+				d = nil
+				return ""
+
 			case "version":
 				d.mu.Lock()
 				s := d.mu.versions.currentVersion().String()

db.go

@@ -1316,6 +1316,12 @@ func (d *DB) Close() error {
 	d.compactionSchedulers.Wait()
 	d.mu.Lock()
 
+	// As a sanity check, ensure that there are no zombie tables. A non-zero count
+	// hints at a reference count leak.
+	if ztbls := len(d.mu.versions.zombieTables); ztbls > 0 {
+		err = firstError(err, errors.Errorf("non-zero zombie file count: %d", ztbls))
+	}
+
 	// If the options include a closer to 'close' the filesystem, close it.
 	if d.opts.private.fsCloser != nil {
 		d.opts.private.fsCloser.Close()

internal/manifest/version.go

@@ -890,6 +890,9 @@ func (v *Version) unrefFiles() []*FileMetadata {
 	for _, lm := range v.Levels {
 		obsolete = append(obsolete, lm.release()...)
 	}
+	for _, lm := range v.RangeKeyLevels {
+		obsolete = append(obsolete, lm.release()...)
+	}
 	return obsolete
 }
 

testdata/compaction_tombstones

@@ -291,3 +291,40 @@ range-deletions-bytes-estimate: 0
 maybe-compact
 ----
 (none)
+
+# Regression test for cockroachdb/cockroach#90149, exercising reference counting
+# on tables that contain a mixture of point, range dels and range keys.
+#
+# Place a table in L6 that contains a RANGEKEYDEL. Because this table is in the
+# bottom of the LSM, and there are no range keys below it, the RANGEKEYDEL is
+# eligible for elision (the RANGEDEL too). After the elision, the input table
+# should be deleted. In #90149, the table still had a reference count, and
+# therefore could not be deleted from the filesystem.
+
+define
+L6
+  rangekey:a-b:{(#1,RANGEKEYDEL)}
+  a.SET.2:a
+  b.SET.3:b
+  c.SET.4:c
+  c.RANGEDEL.5:z
+----
+6:
+  000004:[a#2,SET-z#72057594037927935,RANGEDEL]
+
+wait-pending-table-stats
+000004
+----
+num-entries: 3
+num-deletions: 1
+num-range-key-sets: 0
+point-deletions-bytes-estimate: 0
+range-deletions-bytes-estimate: 39
+
+maybe-compact
+----
+[JOB 100] compacted(elision-only) L6 [000004] (1001 B) + L6 [] (0 B) -> L6 [000005] (778 B), in 1.0s (2.0s total), output rate 778 B/s
+
+# Close the DB, asserting that the reference counts balance.
+close
+----