Skip to content

Commit 1852b7e

Browse files
authored
[Caching] Move cache removal notifications outside lru lock (#14017)
--------- Signed-off-by: Sagar Upadhyaya <[email protected]>
1 parent 9da6170 commit 1852b7e

File tree

3 files changed

+52
-7
lines changed

3 files changed

+52
-7
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4343
- Add ability for Boolean and date field queries to run when only doc_values are enabled ([#11650](https://github.com/opensearch-project/OpenSearch/pull/11650))
4444
- Refactor implementations of query phase searcher, allow QueryCollectorContext to have zero collectors ([#13481](https://github.com/opensearch-project/OpenSearch/pull/13481))
4545
- Adds support to inject telemetry instances to plugins ([#13636](https://github.com/opensearch-project/OpenSearch/pull/13636))
46+
- Move cache removal notifications outside lru lock ([#14017](https://github.com/opensearch-project/OpenSearch/pull/14017))
4647

4748
### Deprecated
4849

server/src/main/java/org/opensearch/common/cache/Cache.java

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
import org.opensearch.common.collect.Tuple;
3737
import org.opensearch.common.util.concurrent.ReleasableLock;
3838

39+
import java.util.ArrayList;
3940
import java.util.Arrays;
4041
import java.util.HashMap;
4142
import java.util.Iterator;
43+
import java.util.List;
4244
import java.util.Map;
4345
import java.util.Objects;
4446
import java.util.concurrent.CompletableFuture;
@@ -396,7 +398,12 @@ private V get(K key, long now, Consumer<Entry<K, V>> onExpiration) {
396398
if (entry == null) {
397399
return null;
398400
} else {
399-
promote(entry, now);
401+
List<RemovalNotification<K, V>> removalNotifications = promote(entry, now).v2();
402+
if (!removalNotifications.isEmpty()) {
403+
for (RemovalNotification<K, V> removalNotification : removalNotifications) {
404+
removalListener.onRemoval(removalNotification);
405+
}
406+
}
400407
return entry.value;
401408
}
402409
}
@@ -446,8 +453,14 @@ private V compute(K key, CacheLoader<K, V> loader) throws ExecutionException {
446453

447454
BiFunction<? super Entry<K, V>, Throwable, ? extends V> handler = (ok, ex) -> {
448455
if (ok != null) {
456+
List<RemovalNotification<K, V>> removalNotifications = new ArrayList<>();
449457
try (ReleasableLock ignored = lruLock.acquire()) {
450-
promote(ok, now);
458+
removalNotifications = promote(ok, now).v2();
459+
}
460+
if (!removalNotifications.isEmpty()) {
461+
for (RemovalNotification<K, V> removalNotification : removalNotifications) {
462+
removalListener.onRemoval(removalNotification);
463+
}
451464
}
452465
return ok.value;
453466
} else {
@@ -512,16 +525,22 @@ private void put(K key, V value, long now) {
512525
CacheSegment<K, V> segment = getCacheSegment(key);
513526
Tuple<Entry<K, V>, Entry<K, V>> tuple = segment.put(key, value, now);
514527
boolean replaced = false;
528+
List<RemovalNotification<K, V>> removalNotifications = new ArrayList<>();
515529
try (ReleasableLock ignored = lruLock.acquire()) {
516530
if (tuple.v2() != null && tuple.v2().state == State.EXISTING) {
517531
if (unlink(tuple.v2())) {
518532
replaced = true;
519533
}
520534
}
521-
promote(tuple.v1(), now);
535+
removalNotifications = promote(tuple.v1(), now).v2();
522536
}
523537
if (replaced) {
524-
removalListener.onRemoval(new RemovalNotification<>(tuple.v2().key, tuple.v2().value, RemovalReason.REPLACED));
538+
removalNotifications.add(new RemovalNotification<>(tuple.v2().key, tuple.v2().value, RemovalReason.REPLACED));
539+
}
540+
if (!removalNotifications.isEmpty()) {
541+
for (RemovalNotification<K, V> removalNotification : removalNotifications) {
542+
removalListener.onRemoval(removalNotification);
543+
}
525544
}
526545
}
527546

@@ -767,8 +786,17 @@ public long getEvictions() {
767786
}
768787
}
769788

770-
private boolean promote(Entry<K, V> entry, long now) {
789+
/**
790+
* Promotes the desired entry to the head of the lru list and tries to see if it needs to evict any entries in
791+
* case the cache size is exceeding or the entry got expired.
792+
* @param entry Entry to be promoted
793+
* @param now the current time
794+
* @return Returns a tuple. v1 signifies whether an entry got promoted, v2 signifies the list of removal
795+
* notifications that the callers needs to handle.
796+
*/
797+
private Tuple<Boolean, List<RemovalNotification<K, V>>> promote(Entry<K, V> entry, long now) {
771798
boolean promoted = true;
799+
List<RemovalNotification<K, V>> removalNotifications = new ArrayList<>();
772800
try (ReleasableLock ignored = lruLock.acquire()) {
773801
switch (entry.state) {
774802
case DELETED:
@@ -782,10 +810,21 @@ private boolean promote(Entry<K, V> entry, long now) {
782810
break;
783811
}
784812
if (promoted) {
785-
evict(now);
813+
while (tail != null && shouldPrune(tail, now)) {
814+
Entry<K, V> entryToBeRemoved = tail;
815+
CacheSegment<K, V> segment = getCacheSegment(entryToBeRemoved.key);
816+
if (segment != null) {
817+
segment.remove(entryToBeRemoved.key, entryToBeRemoved.value, f -> {});
818+
}
819+
if (unlink(entryToBeRemoved)) {
820+
removalNotifications.add(
821+
new RemovalNotification<>(entryToBeRemoved.key, entryToBeRemoved.value, RemovalReason.EVICTED)
822+
);
823+
}
824+
}
786825
}
787826
}
788-
return promoted;
827+
return new Tuple<>(promoted, removalNotifications);
789828
}
790829

791830
private void evict(long now) {

server/src/main/java/org/opensearch/common/cache/RemovalListener.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,10 @@
4242
@ExperimentalApi
4343
@FunctionalInterface
4444
public interface RemovalListener<K, V> {
45+
46+
/**
47+
* This may be called from multiple threads at once. So implementation needs to be thread safe.
48+
* @param notification removal notification for desired entry.
49+
*/
4550
void onRemoval(RemovalNotification<K, V> notification);
4651
}

0 commit comments

Comments
 (0)