Skip to content

Commit 202b4ff

Browse files
committed
Reduce critical section for watchcache.lock
1 parent 834658c commit 202b4ff

File tree

1 file changed

+16
-15
lines changed

1 file changed

+16
-15
lines changed

staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -312,25 +312,26 @@ func (w *watchCache) processEvent(event watch.Event, resourceVersion uint64, upd
312312
RecordTime: w.clock.Now(),
313313
}
314314

315+
// We can call w.store.Get() outside of a critical section,
316+
// because the w.store itself is thread-safe and the only
317+
// place where w.store is modified is below (via updateFunc)
318+
// and these calls are serialized because reflector is processing
319+
// events one-by-one.
320+
previous, exists, err := w.store.Get(elem)
321+
if err != nil {
322+
return err
323+
}
324+
if exists {
325+
previousElem := previous.(*storeElement)
326+
wcEvent.PrevObject = previousElem.Object
327+
wcEvent.PrevObjLabels = previousElem.Labels
328+
wcEvent.PrevObjFields = previousElem.Fields
329+
}
330+
315331
if err := func() error {
316-
// TODO: We should consider moving this lock below after the watchCacheEvent
317-
// is created. In such situation, the only problematic scenario is Replace()
318-
// happening after getting object from store and before acquiring a lock.
319-
// Maybe introduce another lock for this purpose.
320332
w.Lock()
321333
defer w.Unlock()
322334

323-
previous, exists, err := w.store.Get(elem)
324-
if err != nil {
325-
return err
326-
}
327-
if exists {
328-
previousElem := previous.(*storeElement)
329-
wcEvent.PrevObject = previousElem.Object
330-
wcEvent.PrevObjLabels = previousElem.Labels
331-
wcEvent.PrevObjFields = previousElem.Fields
332-
}
333-
334335
w.updateCache(wcEvent)
335336
w.resourceVersion = resourceVersion
336337
defer w.cond.Broadcast()

0 commit comments

Comments
 (0)