Skip to content

Commit f66db20

Browse files
oneonestarebyhr
oneonestar
authored and
ebyhr
committed
Improve database cache and tests
1 parent 24a0a70 commit f66db20

File tree

4 files changed

+54
-58
lines changed

4 files changed

+54
-58
lines changed

‎docs/operation.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ The cache can be configured using the `databaseCache` section in the config file
9393
```yaml
9494
databaseCache:
9595
enabled: true
96-
expireAfterWrite: 60m
96+
expireAfterWrite: 1h
9797
refreshAfterWrite: 5s
9898
```
9999

@@ -105,9 +105,12 @@ Configuration options:
105105
If cache is not refreshed before expiration, requests will fail once the entry
106106
expires (i.e. cache miss will attempt to reload data, but if the database is unavailable,
107107
the request fails because there is no stale value to fall back to after
108-
expiration).
108+
expiration). Default value is `1h`.
109109
* `refreshAfterWrite` - Duration after which cache entries are eligible for
110110
asynchronous refresh. When a refresh is triggered, the existing cached value
111111
continues to be served while the refresh happens in the background.
112112
This helps keep data fresh while serving slightly stale data to avoid blocking requests.
113+
Default value is `5s`.
113114

115+
`expireAfterWrite` and `refreshAfterWrite` can be set to `null` to disable expiration
116+
or refresh respectively.

‎gateway-ha/src/main/java/io/trino/gateway/ha/config/DatabaseCacheConfiguration.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@
1515

1616
import io.airlift.units.Duration;
1717

18-
import java.util.concurrent.TimeUnit;
18+
import static java.util.concurrent.TimeUnit.HOURS;
19+
import static java.util.concurrent.TimeUnit.SECONDS;
1920

2021
public class DatabaseCacheConfiguration
2122
{
2223
private boolean enabled;
23-
private Duration expireAfterWrite = Duration.succinctDuration(60, TimeUnit.MINUTES);
24-
private Duration refreshAfterWrite = Duration.succinctDuration(5, TimeUnit.SECONDS);
24+
private Duration expireAfterWrite = Duration.succinctDuration(1, HOURS);
25+
private Duration refreshAfterWrite = Duration.succinctDuration(5, SECONDS);
2526

2627
public boolean isEnabled()
2728
{

‎gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java‎

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ public class HaGatewayManager
4545

4646
private final GatewayBackendDao dao;
4747
private final String defaultRoutingGroup;
48-
private final boolean cacheEnabled;
4948
private final LoadingCache<Object, List<GatewayBackend>> backendCache;
5049

5150
private final CounterStat backendLookupSuccesses = new CounterStat();
@@ -62,29 +61,28 @@ public HaGatewayManager(Jdbi jdbi, RoutingConfiguration routingConfiguration, Da
6261
{
6362
dao = requireNonNull(jdbi, "jdbi is null").onDemand(GatewayBackendDao.class);
6463
defaultRoutingGroup = routingConfiguration.getDefaultRoutingGroup();
65-
cacheEnabled = databaseCacheConfiguration.isEnabled();
66-
if (cacheEnabled) {
67-
Caffeine<Object, Object> caffeineBuilder = Caffeine.newBuilder()
68-
.initialCapacity(1)
69-
.ticker(ticker);
70-
if (databaseCacheConfiguration.getExpireAfterWrite() != null) {
71-
caffeineBuilder = caffeineBuilder.expireAfterWrite(databaseCacheConfiguration.getExpireAfterWrite().toJavaTime());
72-
}
73-
if (databaseCacheConfiguration.getRefreshAfterWrite() != null) {
74-
caffeineBuilder = caffeineBuilder.refreshAfterWrite(databaseCacheConfiguration.getRefreshAfterWrite().toJavaTime());
75-
}
76-
backendCache = caffeineBuilder.build(this::fetchAllBackends);
7764

78-
// Load the data once during initialization. This ensures a fail-fast behavior in case of database misconfiguration.
79-
try {
80-
List<GatewayBackend> _ = backendCache.get(ALL_BACKEND_CACHE_KEY);
81-
}
82-
catch (Exception e) {
83-
throw new RuntimeException("Failed to warm up backend cache", e);
84-
}
65+
Caffeine<Object, Object> caffeineBuilder = Caffeine.newBuilder()
66+
.initialCapacity(1)
67+
.ticker(ticker);
68+
if (databaseCacheConfiguration.getExpireAfterWrite() != null) {
69+
caffeineBuilder = caffeineBuilder.expireAfterWrite(databaseCacheConfiguration.getExpireAfterWrite().toJavaTime());
8570
}
86-
else {
87-
backendCache = null;
71+
if (databaseCacheConfiguration.getRefreshAfterWrite() != null) {
72+
caffeineBuilder = caffeineBuilder.refreshAfterWrite(databaseCacheConfiguration.getRefreshAfterWrite().toJavaTime());
73+
}
74+
if (!databaseCacheConfiguration.isEnabled()) {
75+
// No-op cache: never stores anything
76+
caffeineBuilder = caffeineBuilder.maximumSize(0);
77+
}
78+
backendCache = caffeineBuilder.build(this::fetchAllBackends);
79+
80+
// Load the data once during initialization. This ensures a fail-fast behavior in case of database misconfiguration.
81+
try {
82+
List<GatewayBackend> _ = backendCache.get(ALL_BACKEND_CACHE_KEY);
83+
}
84+
catch (Exception e) {
85+
throw new RuntimeException("Failed to load gateway backend", e);
8886
}
8987
}
9088

@@ -104,25 +102,18 @@ private List<GatewayBackend> fetchAllBackends(Object ignored)
104102

105103
private void invalidateBackendCache()
106104
{
107-
if (cacheEnabled) {
108-
// Avoid using bulk invalidation like invalidateAll(), in order to invalidate in-flight loads properly.
109-
// See https://github.com/trinodb/trino/issues/10512#issuecomment-1016398117
110-
backendCache.invalidate(ALL_BACKEND_CACHE_KEY);
111-
}
105+
// Avoid using bulk invalidation like invalidateAll(), in order to invalidate in-flight loads properly.
106+
// See https://github.com/trinodb/trino/issues/10512#issuecomment-1016398117
107+
backendCache.invalidate(ALL_BACKEND_CACHE_KEY);
112108
}
113109

114110
private List<GatewayBackend> getAllBackendsInternal()
115111
{
116-
if (cacheEnabled) {
117-
try {
118-
return backendCache.get(ALL_BACKEND_CACHE_KEY);
119-
}
120-
catch (Exception e) {
121-
throw new RuntimeException("Failed to load backends from database to cache", e);
122-
}
112+
try {
113+
return backendCache.get(ALL_BACKEND_CACHE_KEY);
123114
}
124-
else {
125-
return fetchAllBackends(ALL_BACKEND_CACHE_KEY);
115+
catch (Exception e) {
116+
throw new RuntimeException("Failed to load backends from database to cache", e);
126117
}
127118
}
128119

‎gateway-ha/src/test/java/io/trino/gateway/ha/router/TestHaGatewayManager.java‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static io.trino.gateway.ha.TestingJdbcConnectionManager.createTestingJdbcConnectionManager;
3131
import static io.trino.gateway.ha.TestingJdbcConnectionManager.dataStoreConfig;
3232
import static io.trino.gateway.ha.TestingJdbcConnectionManager.destroyTestingDatabase;
33+
import static java.util.concurrent.TimeUnit.SECONDS;
3334
import static org.assertj.core.api.Assertions.assertThat;
3435
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3536

@@ -42,7 +43,7 @@ void testGatewayManagerWithCache()
4243
JdbcConnectionManager connectionManager = createTestingJdbcConnectionManager(dataStoreConfig());
4344
DatabaseCacheConfiguration cacheConfiguration = new DatabaseCacheConfiguration();
4445
cacheConfiguration.setEnabled(true);
45-
cacheConfiguration.setRefreshAfterWrite(new Duration(5, TimeUnit.SECONDS));
46+
cacheConfiguration.setRefreshAfterWrite(new Duration(5, SECONDS));
4647
testGatewayManager(new HaGatewayManager(connectionManager.getJdbi(), new RoutingConfiguration(), cacheConfiguration));
4748
}
4849

@@ -53,7 +54,7 @@ void testGatewayManagerWithoutCache()
5354
testGatewayManager(new HaGatewayManager(connectionManager.getJdbi(), new RoutingConfiguration(), new DatabaseCacheConfiguration()));
5455
}
5556

56-
void testGatewayManager(HaGatewayManager haGatewayManager)
57+
static void testGatewayManager(HaGatewayManager haGatewayManager)
5758
{
5859
ProxyBackendConfiguration backend = new ProxyBackendConfiguration();
5960
backend.setActive(true);
@@ -120,8 +121,8 @@ void testGatewayManagerCacheExpire()
120121
JdbcConnectionManager connectionManager = createTestingJdbcConnectionManager(dataStoreConfig);
121122
DatabaseCacheConfiguration cacheConfiguration = new DatabaseCacheConfiguration();
122123
cacheConfiguration.setEnabled(true);
123-
cacheConfiguration.setRefreshAfterWrite(new Duration(3, TimeUnit.SECONDS));
124-
cacheConfiguration.setExpireAfterWrite(new Duration(5, TimeUnit.SECONDS));
124+
cacheConfiguration.setRefreshAfterWrite(new Duration(3, SECONDS));
125+
cacheConfiguration.setExpireAfterWrite(new Duration(5, SECONDS));
125126
TestingTicker ticker = new TestingTicker();
126127
HaGatewayManager haGatewayManager = new HaGatewayManager(connectionManager.getJdbi(), new RoutingConfiguration(), cacheConfiguration, ticker);
127128

@@ -134,18 +135,18 @@ void testGatewayManagerCacheExpire()
134135
haGatewayManager.addBackend(etl);
135136

136137
// Initial fetch
137-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
138+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo)).hasValue("https://etl1.trino.gateway.io:443");
138139

139-
// Read from cache
140+
// Test read from cache when DB is not available
140141
destroyTestingDatabase(dataStoreConfig);
141-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
142+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo)).hasValue("https://etl1.trino.gateway.io:443");
142143

143144
// Failed to refresh from DB, but still read from cache
144-
ticker.increment(4, TimeUnit.SECONDS);
145-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
145+
ticker.increment(4, SECONDS);
146+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo)).hasValue("https://etl1.trino.gateway.io:443");
146147

147148
// Expired from cache, failed to read from DB
148-
ticker.increment(2, TimeUnit.SECONDS);
149+
ticker.increment(2, SECONDS);
149150
assertThatThrownBy(() -> haGatewayManager.getBackendByName("new-etl1")).hasMessage("Failed to load backends from database to cache");
150151
}
151152

@@ -163,8 +164,8 @@ void testRemoveTrailingSlashInUrl()
163164
etl.setExternalUrl("https://etl1.trino.gateway.io:443/");
164165
haGatewayManager.addBackend(etl);
165166

166-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
167-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl).orElseThrow()).isEqualTo("https://etl1.trino.gateway.io:443");
167+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo)).hasValue("https://etl1.trino.gateway.io:443");
168+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl)).hasValue("https://etl1.trino.gateway.io:443");
168169

169170
ProxyBackendConfiguration etl2 = new ProxyBackendConfiguration();
170171
etl2.setActive(false);
@@ -174,11 +175,11 @@ void testRemoveTrailingSlashInUrl()
174175
etl2.setExternalUrl("https://etl2.trino.gateway.io:443/");
175176
haGatewayManager.updateBackend(etl2);
176177

177-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo).orElseThrow()).isEqualTo("https://etl2.trino.gateway.io:443");
178-
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl).orElseThrow()).isEqualTo("https://etl2.trino.gateway.io:443");
178+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getProxyTo)).hasValue("https://etl2.trino.gateway.io:443");
179+
assertThat(haGatewayManager.getBackendByName("new-etl1").map(ProxyBackendConfiguration::getExternalUrl)).hasValue("https://etl2.trino.gateway.io:443");
179180
}
180181

181-
public static class TestingTicker
182+
private static class TestingTicker
182183
implements Ticker
183184
{
184185
private long time;
@@ -189,10 +190,10 @@ public synchronized long read()
189190
return this.time;
190191
}
191192

192-
public synchronized void increment(long delta, TimeUnit unit)
193+
private synchronized void increment(long delta, TimeUnit unit)
193194
{
194195
checkArgument(delta >= 0L, "delta is negative");
195-
this.time += unit.toNanos(delta);
196+
time += unit.toNanos(delta);
196197
}
197198
}
198199
}

0 commit comments

Comments
 (0)