Skip to content

Commit

Permalink
Drop unnecessary expire time check in ttl event handler
Browse files Browse the repository at this point in the history
handleTTLEvents already calls loadTTLEventKV which gets the most recent
event for the key while holding a lock. This is then checked to confirm
expiration before calling deleteTTLEvent.

Because it's checked before calling, we don't need to do a locked check
of the store AGAIN in deleteTTLEvent. If another goroutine updated the
key in the time between when loadTTLEventKV returned the event, and the
time that deleteTTLEvent tries to delete it, the actual Delete will fail
because the modRevision is no longer current, and the error path will
ensure that the key is re-enqueued and processed at the correct time.

Without this extra check there's not much logic left in deleteTTLEvent,
and there are no other call sites, so just move it into handleTTLEvents.

Signed-off-by: Brad Davidson <[email protected]>
  • Loading branch information
brandond committed Dec 18, 2024
1 parent 7042b49 commit a4169b9
Showing 1 changed file with 8 additions and 19 deletions.
27 changes: 8 additions & 19 deletions pkg/logstructured/logstructured.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,29 +325,18 @@ func (l *LogStructured) handleTTLEvents(ctx context.Context, rwMutex *sync.RWMut
return true
}

l.deleteTTLEvent(ctx, rwMutex, queue, store, eventKV)
return true
}

func (l *LogStructured) deleteTTLEvent(ctx context.Context, rwMutex *sync.RWMutex, queue workqueue.DelayingInterface, store map[string]*ttlEventKV, preEventKV *ttlEventKV) {
logrus.Tracef("TTL delete key=%v, modRev=%v", preEventKV.key, preEventKV.modRevision)
_, _, _, err := l.Delete(ctx, preEventKV.key, preEventKV.modRevision)
logrus.Tracef("TTL delete key=%v, modRev=%v", eventKV.key, eventKV.modRevision)
if _, _, _, err := l.Delete(ctx, eventKV.key, eventKV.modRevision); err != nil {
logrus.Errorf("TTL delete trigger failed for key=%v: %v, requeuing", eventKV.key, err)
queue.AddAfter(eventKV.key, retryInterval)
return true
}

rwMutex.Lock()
defer rwMutex.Unlock()
curEventKV := store[preEventKV.key]
if expires := time.Until(preEventKV.expiredAt); expires > 0 {
logrus.Tracef("TTL changed for key=%v, ttl=%v, requeuing", curEventKV.key, expires)
queue.AddAfter(curEventKV.key, expires)
return
}
if err != nil {
logrus.Errorf("TTL delete trigger failed for key=%v: %v, requeuing", curEventKV.key, err)
queue.AddAfter(curEventKV.key, retryInterval)
return
}
delete(store, eventKV.key)

delete(store, curEventKV.key)
return true
}

// ttlEvents starts a goroutine to do a ListWatch on the root prefix. First it lists
Expand Down

0 comments on commit a4169b9

Please sign in to comment.