Skip to content

Commit 8e78b5a

Browse files
authored
[3.3.2 backport] CBG-5028 Move rev cache unlocks into defer (#7905)
1 parent 9af91e8 commit 8e78b5a

File tree

1 file changed

+22
-19
lines changed

1 file changed

+22
-19
lines changed

db/revision_cache_lru.go

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,26 @@ func (rc *LRURevisionCache) Put(ctx context.Context, docRev DocumentRevision, co
245245

246246
// Upsert a revision in the cache.
247247
func (rc *LRURevisionCache) Upsert(ctx context.Context, docRev DocumentRevision, collectionID uint32) {
248+
248249
key := IDAndRev{DocID: docRev.DocID, RevID: docRev.RevID, CollectionID: collectionID}
250+
numItemsRemoved, value := rc.upsertDocToCache(ctx, key)
251+
if numItemsRemoved > 0 {
252+
rc.cacheNumItems.Add(-numItemsRemoved)
253+
}
254+
255+
docRev.CalculateBytes()
256+
// add new item bytes to overall count
257+
rc.incrRevCacheMemoryUsage(ctx, docRev.MemoryBytes)
258+
value.store(docRev)
259+
260+
// check we aren't over memory capacity, if so perform eviction
261+
rc.revCacheMemoryBasedEviction(ctx)
262+
}
249263

264+
// upsertDocToCache performs the upsert actions requiring the rc lock
265+
func (rc *LRURevisionCache) upsertDocToCache(ctx context.Context, key IDAndRev) (int64, *revCacheValue) {
250266
rc.lock.Lock()
267+
defer rc.lock.Unlock()
251268
newItem := true
252269
// If element exists remove from lrulist
253270
if elem := rc.cache[key]; elem != nil {
@@ -271,18 +288,7 @@ func (rc *LRURevisionCache) Upsert(ctx context.Context, docRev DocumentRevision,
271288
if numBytesEvicted > 0 {
272289
rc._decrRevCacheMemoryUsage(ctx, -numBytesEvicted)
273290
}
274-
rc.lock.Unlock() // release lock after eviction finished
275-
if numItemsRemoved > 0 {
276-
rc.cacheNumItems.Add(-numItemsRemoved)
277-
}
278-
279-
docRev.CalculateBytes()
280-
// add new item bytes to overall count
281-
rc.incrRevCacheMemoryUsage(ctx, docRev.MemoryBytes)
282-
value.store(docRev)
283-
284-
// check we aren't over memory capacity, if so perform eviction
285-
rc.revCacheMemoryBasedEviction(ctx)
291+
return numItemsRemoved, value
286292
}
287293

288294
func (rc *LRURevisionCache) getValue(ctx context.Context, docID, revID string, collectionID uint32, create bool) (value *revCacheValue) {
@@ -292,6 +298,7 @@ func (rc *LRURevisionCache) getValue(ctx context.Context, docID, revID string, c
292298
}
293299
key := IDAndRev{DocID: docID, RevID: revID, CollectionID: collectionID}
294300
rc.lock.Lock()
301+
defer rc.lock.Unlock()
295302
if elem := rc.cache[key]; elem != nil {
296303
rc.lruList.MoveToFront(elem)
297304
value = elem.Value.(*revCacheValue)
@@ -304,14 +311,10 @@ func (rc *LRURevisionCache) getValue(ctx context.Context, docID, revID string, c
304311
if numBytesEvicted > 0 {
305312
rc._decrRevCacheMemoryUsage(ctx, -numBytesEvicted)
306313
}
307-
rc.lock.Unlock() // release lock as eviction is finished
308314
if numItemsRemoved > 0 {
309315
rc.cacheNumItems.Add(-numItemsRemoved)
310316
}
311-
// return early as rev cache mutex has been released at this point
312-
return
313317
}
314-
rc.lock.Unlock()
315318
return
316319
}
317320

@@ -379,6 +382,7 @@ func (value *revCacheValue) load(ctx context.Context, backingStore RevisionCache
379382
value.lock.RUnlock()
380383

381384
value.lock.Lock()
385+
defer value.lock.Unlock()
382386
// Check if the value was loaded while we waited for the lock - if so, return.
383387
if value.bodyBytes != nil || value.err != nil {
384388
cacheHit = true
@@ -397,7 +401,6 @@ func (value *revCacheValue) load(ctx context.Context, backingStore RevisionCache
397401
docRev.CalculateBytes()
398402
value.itemBytes.Store(docRev.MemoryBytes)
399403
}
400-
value.lock.Unlock()
401404

402405
return docRev, cacheHit, err
403406
}
@@ -436,6 +439,7 @@ func (value *revCacheValue) loadForDoc(ctx context.Context, backingStore Revisio
436439
value.lock.RUnlock()
437440

438441
value.lock.Lock()
442+
defer value.lock.Unlock()
439443
// Check if the value was loaded while we waited for the lock - if so, return.
440444
if value.bodyBytes != nil || value.err != nil {
441445
cacheHit = true
@@ -449,13 +453,13 @@ func (value *revCacheValue) loadForDoc(ctx context.Context, backingStore Revisio
449453
docRev.CalculateBytes()
450454
value.itemBytes.Store(docRev.MemoryBytes)
451455
}
452-
value.lock.Unlock()
453456
return docRev, cacheHit, err
454457
}
455458

456459
// Stores a body etc. into a revCacheValue if there isn't one already.
457460
func (value *revCacheValue) store(docRev DocumentRevision) {
458461
value.lock.Lock()
462+
defer value.lock.Unlock()
459463
if value.bodyBytes == nil {
460464
// value already has doc id/rev id in key
461465
value.bodyBytes = docRev.BodyBytes
@@ -467,7 +471,6 @@ func (value *revCacheValue) store(docRev DocumentRevision) {
467471
value.err = nil
468472
value.itemBytes.Store(docRev.MemoryBytes)
469473
}
470-
value.lock.Unlock()
471474
}
472475

473476
func (value *revCacheValue) updateDelta(toDelta RevisionDelta) (diffInBytes int64) {

0 commit comments

Comments
 (0)