Skip to content
This repository has been archived by the owner on Jul 27, 2023. It is now read-only.

Add minDelayOnEmptyResult in cache config #360

Merged
merged 1 commit into from
Nov 28, 2018
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
10 changes: 8 additions & 2 deletions src/main/java/com/orbitz/consul/cache/ConsulCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ enum State {latent, starting, started, stopped }
private final ClientEventHandler eventHandler;
private final CacheDescriptor cacheDescriptor;

ConsulCache(
protected ConsulCache(
Function<V, K> keyConversion,
CallbackConsumer<V> callbackConsumer,
CacheConfig cacheConfig,
Expand Down Expand Up @@ -124,7 +124,13 @@ public void onComplete(ConsulResponse<List<V>> consulResponse) {
initLatch.countDown();
}

Duration timeToWait = cacheConfig.getMinimumDurationBetweenRequests().minus(elapsedTime);
Duration timeToWait = cacheConfig.getMinimumDurationBetweenRequests();
if ((consulResponse.getResponse() == null || consulResponse.getResponse().isEmpty()) &&
cacheConfig.getMinimumDurationDelayOnEmptyResult().compareTo(timeToWait) > 0) {
timeToWait = cacheConfig.getMinimumDurationDelayOnEmptyResult();
}
timeToWait = timeToWait.minus(elapsedTime);

if (timeToWait.isNegative() || timeToWait.isZero()) {
runCallback();
} else {
Expand Down
26 changes: 23 additions & 3 deletions src/main/java/com/orbitz/consul/config/CacheConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public class CacheConfig {
@VisibleForTesting
static final Duration DEFAULT_MIN_DELAY_BETWEEN_REQUESTS = Duration.ZERO;
@VisibleForTesting
static final Duration DEFAULT_MIN_DELAY_ON_EMPTY_RESULT = Duration.ZERO;
@VisibleForTesting
static final boolean DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_ENABLED = true;
@VisibleForTesting
static final Duration DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_MARGIN = Duration.ofSeconds(2);
Expand All @@ -24,16 +26,18 @@ public class CacheConfig {
private final Duration minBackOffDelay;
private final Duration maxBackOffDelay;
private final Duration minDelayBetweenRequests;
private final Duration minDelayOnEmptyResult;
private final Duration timeoutAutoAdjustmentMargin;
private final boolean timeoutAutoAdjustmentEnabled;
private final RefreshErrorLogConsumer refreshErrorLogConsumer;

private CacheConfig(Duration minBackOffDelay, Duration maxBackOffDelay, Duration minDelayBetweenRequests,
boolean timeoutAutoAdjustmentEnabled, Duration timeoutAutoAdjustmentMargin,
RefreshErrorLogConsumer refreshErrorLogConsumer) {
Duration minDelayOnEmptyResult, boolean timeoutAutoAdjustmentEnabled,
Duration timeoutAutoAdjustmentMargin, RefreshErrorLogConsumer refreshErrorLogConsumer) {
this.minBackOffDelay = minBackOffDelay;
this.maxBackOffDelay = maxBackOffDelay;
this.minDelayBetweenRequests = minDelayBetweenRequests;
this.minDelayOnEmptyResult = minDelayOnEmptyResult;
this.timeoutAutoAdjustmentEnabled = timeoutAutoAdjustmentEnabled;
this.timeoutAutoAdjustmentMargin = timeoutAutoAdjustmentMargin;
this.refreshErrorLogConsumer = refreshErrorLogConsumer;
Expand Down Expand Up @@ -82,6 +86,13 @@ public Duration getMinimumDurationBetweenRequests() {
return minDelayBetweenRequests;
}

/**
* Gets the minimum time between two requests for caches.
*/
public Duration getMinimumDurationDelayOnEmptyResult() {
return minDelayOnEmptyResult;
}

/**
* Gets the function that will be called in case of error.
*/
Expand All @@ -102,6 +113,7 @@ public static class Builder {
private Duration minBackOffDelay = DEFAULT_BACKOFF_DELAY;
private Duration maxBackOffDelay = DEFAULT_BACKOFF_DELAY;
private Duration minDelayBetweenRequests = DEFAULT_MIN_DELAY_BETWEEN_REQUESTS;
private Duration minDelayOnEmptyResult = DEFAULT_MIN_DELAY_ON_EMPTY_RESULT;
private Duration timeoutAutoAdjustmentMargin = DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_MARGIN;
private boolean timeoutAutoAdjustmentEnabled = DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_ENABLED;
private RefreshErrorLogConsumer refreshErrorLogConsumer = DEFAULT_REFRESH_ERROR_LOG_CONSUMER;
Expand Down Expand Up @@ -141,6 +153,14 @@ public Builder withMinDelayBetweenRequests(Duration delay) {
return this;
}

/**
* Sets the minimum time between two requests for caches when an empty result is returned.
*/
public Builder withMinDelayOnEmptyResult(Duration delay) {
this.minDelayOnEmptyResult = Preconditions.checkNotNull(delay, "Delay cannot be null");
return this;
}

/**
* Enable/Disable the automatic adjustment of read timeout
*/
Expand Down Expand Up @@ -183,7 +203,7 @@ public Builder withRefreshErrorLoggedAs(RefreshErrorLogConsumer fn) {
}

public CacheConfig build() {
return new CacheConfig(minBackOffDelay, maxBackOffDelay, minDelayBetweenRequests,
return new CacheConfig(minBackOffDelay, maxBackOffDelay, minDelayBetweenRequests, minDelayOnEmptyResult,
timeoutAutoAdjustmentEnabled, timeoutAutoAdjustmentMargin,
refreshErrorLogConsumer);
}
Expand Down
98 changes: 98 additions & 0 deletions src/test/java/com/orbitz/consul/config/CacheConfigTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package com.orbitz.consul.config;

import com.orbitz.consul.cache.CacheDescriptor;
import com.orbitz.consul.cache.ConsulCache;
import com.orbitz.consul.model.ConsulResponse;
import com.orbitz.consul.monitoring.ClientEventHandler;
import junitparams.JUnitParamsRunner;
import junitparams.Parameters;
import junitparams.naming.TestCaseName;
Expand All @@ -8,8 +12,15 @@
import org.junit.runner.RunWith;
import org.slf4j.Logger;

import java.math.BigInteger;
import java.time.Duration;
import java.time.LocalTime;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Supplier;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
Expand All @@ -28,6 +39,7 @@ public void testDefaults() {
assertEquals(CacheConfig.DEFAULT_BACKOFF_DELAY, config.getMaximumBackOffDelay());
assertEquals(CacheConfig.DEFAULT_WATCH_DURATION, config.getWatchDuration());
assertEquals(CacheConfig.DEFAULT_MIN_DELAY_BETWEEN_REQUESTS, config.getMinimumDurationBetweenRequests());
assertEquals(CacheConfig.DEFAULT_MIN_DELAY_ON_EMPTY_RESULT, config.getMinimumDurationDelayOnEmptyResult());
assertEquals(CacheConfig.DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_ENABLED, config.isTimeoutAutoAdjustmentEnabled());
assertEquals(CacheConfig.DEFAULT_TIMEOUT_AUTO_ADJUSTMENT_MARGIN, config.getTimeoutAutoAdjustmentMargin());

Expand Down Expand Up @@ -58,6 +70,14 @@ public void testOverrideMinDelayBetweenRequests(Duration delayBetweenRequests) {
assertEquals(delayBetweenRequests, config.getMinimumDurationBetweenRequests());
}

@Test
@Parameters(method = "getDurationSamples")
@TestCaseName("Delay: {0}")
public void testOverrideMinDelayOnEmptyResult(Duration delayBetweenRequests) {
CacheConfig config = CacheConfig.builder().withMinDelayOnEmptyResult(delayBetweenRequests).build();
assertEquals(delayBetweenRequests, config.getMinimumDurationDelayOnEmptyResult());
}

@Test
@Parameters({"true", "false"})
@TestCaseName("Enabled: {0}")
Expand Down Expand Up @@ -156,4 +176,82 @@ public Object getMinMaxDurationSamples() {
new Object[] { Duration.ofSeconds(-1), Duration.ofSeconds(-1), false },
};
}

@Test
public void testMinDelayOnEmptyResultWithNoResults() throws InterruptedException {
TestCacheSupplier res = new TestCacheSupplier(0, Duration.ofMillis(100));

TestCache cache = TestCache.createCache(CacheConfig.builder()
.withMinDelayOnEmptyResult(Duration.ofMillis(100))
.build(), res);
cache.start();
Thread.sleep(300);
assertTrue(res.run > 0);
cache.stop();
}

@Test
public void testMinDelayOnEmptyResultWithResults() throws InterruptedException {
TestCacheSupplier res = new TestCacheSupplier(1, Duration.ofMillis(50));

TestCache cache = TestCache.createCache(CacheConfig.builder()
.withMinDelayOnEmptyResult(Duration.ofMillis(100))
.withMinDelayBetweenRequests(Duration.ofMillis(50)) // do not blow ourselves up
.build(), res);
cache.start();
Thread.sleep(300);
assertTrue(res.run > 0);
cache.stop();
}


static class TestCache extends ConsulCache<Integer, Integer> {
private TestCache(Function<Integer, Integer> keyConversion, CallbackConsumer<Integer> callbackConsumer, CacheConfig cacheConfig, ClientEventHandler eventHandler, CacheDescriptor cacheDescriptor) {
super(keyConversion, callbackConsumer, cacheConfig, eventHandler, cacheDescriptor);
}

static TestCache createCache(CacheConfig config, Supplier<List<Integer>> res) {
ClientEventHandler ev = mock(ClientEventHandler.class);
CacheDescriptor cacheDescriptor = new CacheDescriptor("test", "test");

final CallbackConsumer<Integer> callbackConsumer = (index, callback) -> {
callback.onComplete(new ConsulResponse<>(res.get(), 0, true, BigInteger.ZERO));
};

return new TestCache((i) -> i,
callbackConsumer,
config,
ev,
cacheDescriptor);
}
}

static class TestCacheSupplier implements Supplier<List<Integer>> {
int run = 0;
int resultCount;
private Duration expectedInterval;
private LocalTime lastCall;

TestCacheSupplier(int resultCount, Duration expectedInterval) {
this.resultCount = resultCount;
this.expectedInterval = expectedInterval;
}

@Override
public List<Integer> get() {
if (lastCall != null) {
long between = Duration.between(lastCall, LocalTime.now()).toMillis();
assertTrue(String.format("expected duration between calls of %d, got %s", expectedInterval.toMillis(), between),
Math.abs(between - expectedInterval.toMillis()) < 20);
}
lastCall = LocalTime.now();
run++;

List<Integer> response = new ArrayList<>();
for (int i = 0; i < resultCount; i++) {
response.add(1);
}
return response;
}
}
}