From 9ad74adaaa62c71041bfa8f933bc163cc8878398 Mon Sep 17 00:00:00 2001
From: Jackson Owens <jackson@cockroachlabs.com>
Date: Wed, 26 Jul 2023 10:40:48 -0400
Subject: [PATCH] internal/cache: periodically drop mutex during EvictFile

We've observed significant block cache mutex contention originating from
EvictFile. When evicting a file with a significant count of blocks currently
held within the block cache, EvictFile may hold the block cache shard mutex for
a long duration, increasing latency for requests that must access the same
shard. This commit periodically drops the mutex to provide these other requests
with an opportunity to make progress.

Informs #1997.
---
 internal/cache/clockpro.go | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/internal/cache/clockpro.go b/internal/cache/clockpro.go
index 94102a274ba..311cdd39358 100644
--- a/internal/cache/clockpro.go
+++ b/internal/cache/clockpro.go
@@ -247,23 +247,35 @@ func (c *shard) Delete(id uint64, fileNum base.DiskFileNum, offset uint64) {
 
 // EvictFile evicts all of the cache values for the specified file.
 func (c *shard) EvictFile(id uint64, fileNum base.DiskFileNum) {
+	fkey := key{fileKey{id, fileNum}, 0}
+	for c.evictFileRun(fkey) {
+	}
+}
+
+func (c *shard) evictFileRun(fkey key) (moreRemaining bool) {
+	// If most of the file's blocks are held in the block cache, evicting all
+	// the blocks may take a while. We don't want to block the entire cache
+	// shard, forcing concurrent readers until we're finished. We drop the mutex
+	// every [blocksPerMutexAcquisition] blocks to give other goroutines an
+	// opportunity to make progress.
+	const blocksPerMutexAcquisition = 5
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	fkey := key{fileKey{id, fileNum}, 0}
 	blocks := c.files.Get(fkey)
 	if blocks == nil {
-		return
+		return false
 	}
-	for b, n := blocks, (*entry)(nil); ; b = n {
+	for i, b, n := 0, blocks, (*entry)(nil); i < blocksPerMutexAcquisition; i, b = i+1, n {
 		n = b.fileLink.next
 		c.metaEvict(b)
 		if b == n {
-			break
+			c.checkConsistency()
+			return false
 		}
 	}
-
-	c.checkConsistency()
+	// Exhausted blocksPerMutexAcquisition.
+	return true
 }
 
 func (c *shard) Free() {