Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added overall timeout to evictable cache #2659

Merged
merged 1 commit into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
* Default implementation is backed by {@link java.util.concurrent.ConcurrentHashMap} and provides
* configuration to set this map up, as can be done through {@link #builder()}, and {@link #create(io.helidon.config.Config)}.
*
* Cache timeouts:
* <ul>
* <li>{@link io.helidon.security.providers.common.EvictableCache.Builder#overallTimeout(long, java.util.concurrent.TimeUnit)}
* defines the timeout of record since its creation</li>
* <li>{@link io.helidon.security.providers.common.EvictableCache.Builder#timeout(long, java.util.concurrent.TimeUnit)}
* defines the timeout of record since last use (a sliding timeout)</li>
* </ul>
*
* @param <K> type of keys in this cache
* @param <V> type of values in this cache
*/
Expand Down Expand Up @@ -180,8 +188,10 @@ default void close() {
class Builder<K, V> implements io.helidon.common.Builder<EvictableCache<K, V>> {
private boolean cacheEnabled = true;
private long cacheTimeout = CACHE_TIMEOUT_MINUTES;
private long cacheMaxSize = CACHE_MAX_SIZE;
private TimeUnit cacheTimeoutUnit = TimeUnit.MINUTES;
private long overallTimeout = CACHE_TIMEOUT_MINUTES;
private TimeUnit overallTimeoutUnit = TimeUnit.MINUTES;
private long cacheMaxSize = CACHE_MAX_SIZE;
private long cacheEvictDelay = CACHE_EVICT_DELAY_MINUTES;
private long cacheEvictPeriod = CACHE_EVICT_PERIOD_MINUTES;
private TimeUnit cacheEvictTimeUnit = TimeUnit.MINUTES;
Expand All @@ -203,7 +213,7 @@ public EvictableCache<K, V> build() {
}

/**
* Configure record timeout since last modification.
* Configure record timeout since last access.
*
* @param timeout timeout value
* @param timeoutUnit timeout unit
Expand All @@ -215,6 +225,19 @@ public Builder<K, V> timeout(long timeout, TimeUnit timeoutUnit) {
return this;
}

/**
* Configure record timeout since its creation.
*
* @param timeout timeout value
* @param timeoutUnit timeout unit
* @return updated builder instance
*/
public Builder<K, V> overallTimeout(long timeout, TimeUnit timeoutUnit) {
this.overallTimeout = timeout;
this.overallTimeoutUnit = timeoutUnit;
return this;
}

/**
* Configure maximal cache size.
*
Expand Down Expand Up @@ -306,6 +329,8 @@ public Builder<K, V> config(Config config) {
if (cacheEnabled) {
config.get("max-size").asInt().ifPresent(this::maxSize);
config.get("cache-timeout-millis").asLong().ifPresent(timeout -> timeout(timeout, TimeUnit.MILLISECONDS));
config.get("cache-overall-timeout-millis").asLong()
.ifPresent(timeout -> overallTimeout(timeout, TimeUnit.MILLISECONDS));
long evictDelay = config.get("cache-evict-delay-millis").asLong()
.orElse(cacheEvictTimeUnit.toMillis(cacheEvictDelay));
long evictPeriod = config.get("cache-evict-period-millis").asLong()
Expand Down Expand Up @@ -335,14 +360,22 @@ long cacheTimeout() {
return cacheTimeout;
}

long cacheMaxSize() {
return cacheMaxSize;
}

TimeUnit cacheTimeoutUnit() {
return cacheTimeoutUnit;
}

long overallTimeout() {
return overallTimeout;
}

TimeUnit overallTimeoutUnit() {
return overallTimeoutUnit;
}

long cacheMaxSize() {
return cacheMaxSize;
}

long cacheEvictDelay() {
return cacheEvictDelay;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -54,15 +54,17 @@ public Thread newThread(Runnable r) {
}

private final ConcurrentHashMap<K, CacheRecord<K, V>> cacheMap = new ConcurrentHashMap<>();
private final long cacheTimoutNanos;
private final long cacheTimeoutNanos;
private final long overallTimeoutNanos;
private final long cacheMaxSize;
private final long evictParallelismThreshold;
private final ScheduledFuture<?> evictionFuture;
private final BiFunction<K, V, Boolean> evictor;

EvictableCacheImpl(Builder<K, V> builder) {
cacheMaxSize = builder.cacheMaxSize();
cacheTimoutNanos = TimeUnit.NANOSECONDS.convert(builder.cacheTimeout(), builder.cacheTimeoutUnit());
cacheTimeoutNanos = TimeUnit.NANOSECONDS.convert(builder.cacheTimeout(), builder.cacheTimeoutUnit());
overallTimeoutNanos = TimeUnit.NANOSECONDS.convert(builder.overallTimeout(), builder.overallTimeoutUnit());
evictParallelismThreshold = builder.parallelismThreshold();
evictor = builder.evictor();

Expand Down Expand Up @@ -114,7 +116,7 @@ void evict() {
if ((null == cacheRecord) || evictor.apply(cacheRecord.getKey(), cacheRecord.getValue())) {
return null;
} else {
if (cacheRecord.isValid(cacheTimoutNanos)) {
if (cacheRecord.isValid(cacheTimeoutNanos, overallTimeoutNanos)) {
return cacheRecord;
} else {
return null;
Expand All @@ -124,7 +126,7 @@ void evict() {
}

private Optional<CacheRecord<K, V>> validate(CacheRecord<K, V> record) {
if (record.isValid(cacheTimoutNanos) && !evictor.apply(record.getKey(), record.getValue())) {
if (record.isValid(cacheTimeoutNanos, overallTimeoutNanos) && !evictor.apply(record.getKey(), record.getValue())) {
return Optional.of(record);
}
cacheMap.remove(record.key);
Expand All @@ -133,7 +135,7 @@ private Optional<CacheRecord<K, V>> validate(CacheRecord<K, V> record) {

private Optional<V> doComputeValue(K key, Supplier<Optional<V>> valueSupplier) {
CacheRecord<K, V> record = cacheMap.compute(key, (s, cacheRecord) -> {
if ((null != cacheRecord) && cacheRecord.isValid(cacheTimoutNanos)) {
if ((null != cacheRecord) && cacheRecord.isValid(cacheTimeoutNanos, overallTimeoutNanos)) {
cacheRecord.accessed();
return cacheRecord;
}
Expand Down Expand Up @@ -161,6 +163,7 @@ private Optional<CacheRecord<K, V>> getRecord(K key) {
private static final class CacheRecord<K, V> {
private final K key;
private final V value;
private final long created = System.nanoTime();
private volatile long lastAccess = System.nanoTime();

private CacheRecord(K key, V value) {
Expand All @@ -172,8 +175,10 @@ private void accessed() {
lastAccess = System.nanoTime();
}

private boolean isValid(long timeoutNanos) {
return (System.nanoTime() - lastAccess) < timeoutNanos;
private boolean isValid(long timeoutNanos, long overallTimeout) {
long nano = System.nanoTime();

return ((nano - created) < overallTimeout) && ((nano - lastAccess) < timeoutNanos);
}

private K getKey() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -63,6 +63,21 @@ void testEviction() throws InterruptedException {
cache.close();
}

@Test
void testOverallTimeout() throws InterruptedException {
EvictableCache<String, String> cache = EvictableCache.<String, String>builder()
.timeout(10, TimeUnit.MINUTES)
.overallTimeout(50, TimeUnit.MILLISECONDS)
.build();

assertThat(cache.computeValue("one", () -> Optional.of("1")), is(Optional.of("1")));
assertThat(cache.get("one"), is(Optional.of("1")));
TimeUnit.MILLISECONDS.sleep(200);
assertThat(cache.get("one"), is(EMPTY));

cache.close();
}

@Test
void testEvictor() {
EvictableCache<String, String> cache = EvictableCache.<String, String>builder()
Expand Down