crl-release-23.1: manifest: grab manifest lock when calling InitCompa…

…ctingFileInfo

Previously, we had one case where we could be calling
InitCompactingFileInfo (a method on L0Sublevels that must be called
on the most-recently-created L0Sublevels) on a failed compaction
while another version was being installed but was in the process
of writing to the manifest, and had hence dropped the db mutex
during IO. This had the potential to panic on a case where we
expect the interval indices in FileMetadata to match exactly
with those in the latest version's L0Sublevels. If there's a newer
L0Sublevels being created that hasn't been installed into the
versions list yet, the two are expected to be out of sync.

This change resolves this by grabbing the manifest lock before
calling InitCompactingFileInfo in clearCompactionState.

Will address cockroachdb/cockroach#109866 when it lands in
cockroach.

compaction.go

@@ -1633,8 +1633,9 @@ func (d *DB) addInProgressCompaction(c *compaction) {
 // indicates whether the compaction state should be rolled back to its original
 // state in the case of an unsuccessful compaction.
 //
-// DB.mu must be held when calling this method. All writes to the manifest for
-// this compaction should have completed by this point.
+// DB.mu must be held when calling this method, however this method can drop and
+// re-acquire that mutex. All writes to the manifest for this compaction should
+// have completed by this point.
 func (d *DB) removeInProgressCompaction(c *compaction, rollback bool) {
 	for _, cl := range c.inputs {
 		iter := cl.files.Iter()
@@ -1662,7 +1663,17 @@ func (d *DB) removeInProgressCompaction(c *compaction, rollback bool) {
 	delete(d.mu.compact.inProgress, c)
 
 	l0InProgress := inProgressL0Compactions(d.getInProgressCompactionInfoLocked(c))
-	d.mu.versions.currentVersion().L0Sublevels.InitCompactingFileInfo(l0InProgress)
+	func() {
+		// InitCompactingFileInfo requires that no other manifest writes be
+		// happening in parallel with it, i.e. we're not in the midst of installing
+		// another version. Otherwise, it's possible that we've created another
+		// L0Sublevels instance, but not added it to the versions list, causing
+		// all the indices in FileMetadata to be inaccurate. To ensure this,
+		// grab the manifest lock.
+		d.mu.versions.logLock()
+		defer d.mu.versions.logUnlock()
+		d.mu.versions.currentVersion().L0Sublevels.InitCompactingFileInfo(l0InProgress)
+	}()
 }
 
 func (d *DB) calculateDiskAvailableBytes() uint64 {

internal/manifest/l0_sublevels.go

@@ -712,7 +712,7 @@ func (s *L0Sublevels) calculateFlushSplitKeys(flushSplitMaxBytes int64) {
 // InitCompactingFileInfo initializes internal flags relating to compacting
 // files. Must be called after sublevel initialization.
 //
-// Requires DB.mu to be held.
+// Requires DB.mu *and* the manifest lock to be held.
 func (s *L0Sublevels) InitCompactingFileInfo(inProgress []L0Compaction) {
 	for i := range s.orderedIntervals {
 		s.orderedIntervals[i].compactingFileCount = 0
@@ -735,6 +735,13 @@ func (s *L0Sublevels) InitCompactingFileInfo(inProgress []L0Compaction) {
 		if !f.IsCompacting() {
 			continue
 		}
+		if invariants.Enabled {
+			if s.cmp(s.orderedIntervals[f.minIntervalIndex].startKey.key, f.Smallest.UserKey) != 0 || s.cmp(s.orderedIntervals[f.maxIntervalIndex+1].startKey.key, f.Largest.UserKey) != 0 {
+				panic(fmt.Sprintf("file %s has inconsistent L0 Sublevel interval bounds: %s-%s, %s-%s", f.FileNum,
+					s.orderedIntervals[f.minIntervalIndex].startKey.key, s.orderedIntervals[f.maxIntervalIndex+1].startKey.key,
+					f.Smallest.UserKey, f.Largest.UserKey))
+			}
+		}
 		for i := f.minIntervalIndex; i <= f.maxIntervalIndex; i++ {
 			interval := &s.orderedIntervals[i]
 			interval.compactingFileCount++