Skip to content

Commit 3c872e7

Browse files
mp911demp911de
committed
Polishing
Rename LimitedStatementCache to BoundedStatementCache. Synchronize cache access. Update documentation. Extract PgBouncerIntegrationTests into its own top-level class. [pgjdbc#223][pgjdbc#225]
1 parent cc48b6d commit 3c872e7

13 files changed

+253
-131
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ Mono<Connection> connectionMono = Mono.from(connectionFactory.create());
7878
| `applicationName` | The name of the application connecting to the database. Defaults to `r2dbc-postgresql`. _(Optional)_
7979
| `autodetectExtensions` | Whether to auto-detect and register `Extension`s from the class path. Defaults to `true`. _(Optional)_
8080
| `forceBinary` | Whether to force binary transfer. Defaults to `false`. _(Optional)_
81+
| `preparedStatementCacheQueries` | Determine the number of queries that are cached in each connection. The default is `-1`, meaning there's no limit. The value of `-1` disables the cache. Any other value specifies the cache size.
8182
| `options` | A `Map<String, String>` of connection parameters. These are applied to each database connection created by the `ConnectionFactory`. Useful for setting generic [PostgreSQL connection parameters][psql-runtime-config]. _(Optional)_
8283
| `schema` | The schema to set. _(Optional)_
8384
| `sslMode` | SSL mode to use, see `SSLMode` enum. Supported values: `DISABLE`, `ALLOW`, `PREFER`, `REQUIRE`, `VERIFY_CA`, `VERIFY_FULL`. _(Optional)_

‎src/main/java/io/r2dbc/postgresql/LimitedStatementCache.java renamed to ‎src/main/java/io/r2dbc/postgresql/BoundedStatementCache.java

+76-7
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,32 @@
2121
import io.r2dbc.postgresql.client.ExtendedQueryMessageFlow;
2222
import io.r2dbc.postgresql.util.Assert;
2323
import reactor.core.publisher.Mono;
24+
import reactor.util.annotation.Nullable;
2425
import reactor.util.function.Tuple2;
2526
import reactor.util.function.Tuples;
2627

28+
import java.util.ArrayList;
29+
import java.util.Collection;
30+
import java.util.Iterator;
2731
import java.util.LinkedHashMap;
2832
import java.util.List;
33+
import java.util.Map;
2934
import java.util.concurrent.atomic.AtomicInteger;
3035

31-
public class LimitedStatementCache implements StatementCache {
36+
/**
37+
* Bounded (size-limited) {@link StatementCache}.
38+
*/
39+
final class BoundedStatementCache implements StatementCache {
3240

33-
private final LinkedHashMap<Tuple2<String, List<Integer>>, String> cache = new LinkedHashMap<>(16, 0.75f, true);
41+
private final Map<Tuple2<String, List<Integer>>, String> cache = new LinkedHashMap<>(16, 0.75f, true);
3442

3543
private final Client client;
3644

3745
private final AtomicInteger counter = new AtomicInteger();
3846

3947
private final int limit;
4048

41-
public LimitedStatementCache(Client client, int limit) {
49+
public BoundedStatementCache(Client client, int limit) {
4250
this.client = Assert.requireNonNull(client, "client must not be null");
4351
if (limit <= 0) {
4452
throw new IllegalArgumentException("statement cache limit must be greater than zero");
@@ -51,16 +59,16 @@ public Mono<String> getName(Binding binding, String sql) {
5159
Assert.requireNonNull(binding, "binding must not be null");
5260
Assert.requireNonNull(sql, "sql must not be null");
5361
Tuple2<String, List<Integer>> key = Tuples.of(sql, binding.getParameterTypes());
54-
String name = this.cache.get(key);
62+
String name = get(key);
5563
if (name != null) {
5664
return Mono.just(name);
5765
}
5866

5967
Mono<Void> closeLastStatement = Mono.defer(() -> {
60-
if (this.cache.size() < this.limit) {
68+
if (getCacheSize() < this.limit) {
6169
return Mono.empty();
6270
}
63-
String lastAccessedStatementName = this.cache.values().iterator().next();
71+
String lastAccessedStatementName = getAndRemoveEldest();
6472
ExceptionFactory factory = ExceptionFactory.withSql(lastAccessedStatementName);
6573
return ExtendedQueryMessageFlow
6674
.closeStatement(this.client, lastAccessedStatementName)
@@ -69,7 +77,68 @@ public Mono<String> getName(Binding binding, String sql) {
6977
});
7078

7179
return closeLastStatement.then(this.parse(sql, binding.getParameterTypes()))
72-
.doOnNext(preparedName -> this.cache.put(key, preparedName));
80+
.doOnNext(preparedName -> put(key, preparedName));
81+
}
82+
83+
84+
/**
85+
* Synchronized cache access: Return all statement names.
86+
*
87+
* @return statement names.
88+
*/
89+
Collection<String> getCachedStatementNames() {
90+
synchronized (this.cache) {
91+
List<String> names = new ArrayList<>(this.cache.size());
92+
names.addAll(cache.values());
93+
return names;
94+
}
95+
}
96+
97+
/**
98+
* Synchronized cache access: Retrieve statement name by key.
99+
*
100+
* @param key
101+
* @return statement name by key
102+
*/
103+
@Nullable
104+
private String get(Tuple2<String, List<Integer>> key) {
105+
synchronized (this.cache) {
106+
return this.cache.get(key);
107+
}
108+
}
109+
110+
/**
111+
* Synchronized cache access: Least recently used entry.
112+
*
113+
* @return least recently used entry
114+
*/
115+
private String getAndRemoveEldest() {
116+
synchronized (this.cache) {
117+
Iterator<Map.Entry<Tuple2<String, List<Integer>>, String>> iterator = this.cache.entrySet().iterator();
118+
String entry = iterator.next().getValue();
119+
iterator.remove();
120+
return entry;
121+
}
122+
}
123+
124+
/**
125+
* Synchronized cache access: Store prepared statement.
126+
*/
127+
private void put(Tuple2<String, List<Integer>> key, String preparedName) {
128+
synchronized (this.cache) {
129+
this.cache.put(key, preparedName);
130+
}
131+
}
132+
133+
/**
134+
* Synchronized cache access: Return the cache size.
135+
*
136+
* @return the cache size.
137+
*/
138+
private int getCacheSize() {
139+
synchronized (this.cache) {
140+
return this.cache.size();
141+
}
73142
}
74143

75144
@Override

‎src/main/java/io/r2dbc/postgresql/IndefiniteStatementCache.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ public Mono<String> getName(Binding binding, String sql) {
4646
Assert.requireNonNull(binding, "binding must not be null");
4747
Assert.requireNonNull(sql, "sql must not be null");
4848

49-
return this.cache.computeIfAbsent(Tuples.of(sql, binding.getParameterTypes()),
50-
tuple -> this.parse(tuple.getT1(), tuple.getT2()));
49+
synchronized (this.cache) {
50+
return this.cache.computeIfAbsent(Tuples.of(sql, binding.getParameterTypes()),
51+
tuple -> this.parse(tuple.getT1(), tuple.getT2()));
52+
}
5153
}
5254

5355
@Override

‎src/main/java/io/r2dbc/postgresql/PostgresqlConnectionConfiguration.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -579,11 +579,12 @@ public Builder username(String username) {
579579
}
580580

581581
/**
582-
* Configure the preparedStatementCacheQueries.
582+
* Configure the preparedStatementCacheQueries. The default is {@code -1}, meaning there's no limit. The value of {@code 0} disables the cache. Any other value specifies the cache size.
583583
*
584584
* @param preparedStatementCacheQueries the preparedStatementCacheQueries
585585
* @return this {@link Builder}
586586
* @throws IllegalArgumentException if {@code username} is {@code null}
587+
* @since 0.8.1
587588
*/
588589
public Builder preparedStatementCacheQueries(int preparedStatementCacheQueries) {
589590
this.preparedStatementCacheQueries = preparedStatementCacheQueries;

‎src/main/java/io/r2dbc/postgresql/PostgresqlConnectionFactoryProvider.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.r2dbc.postgresql;
1818

1919
import io.netty.handler.ssl.SslContextBuilder;
20+
import io.r2dbc.postgresql.client.DefaultHostnameVerifier;
2021
import io.r2dbc.postgresql.client.SSLMode;
2122
import io.r2dbc.postgresql.util.Assert;
2223
import io.r2dbc.spi.ConnectionFactoryOptions;
@@ -87,7 +88,7 @@ public final class PostgresqlConnectionFactoryProvider implements ConnectionFact
8788
public static final Option<String> SSL_CERT = Option.valueOf("sslCert");
8889

8990
/**
90-
* Class name of hostname verifier. Defaults to using io.r2dbc.postgresql.client.PGHostnameVerifier
91+
* Class name of hostname verifier. Defaults to {@link DefaultHostnameVerifier}.
9192
*/
9293
public static final Option<HostnameVerifier> SSL_HOSTNAME_VERIFIER = Option.valueOf("sslHostnameVerifier");
9394

@@ -113,7 +114,7 @@ public final class PostgresqlConnectionFactoryProvider implements ConnectionFact
113114

114115
/**
115116
* Determine the number of queries that are cached in each connection.
116-
* The default is -1, meaning there's no limit. The value of 0 disables the cache.
117+
* The default is {@code -1}, meaning there's no limit. The value of {@code 0} disables the cache. Any other value specifies the cache size.
117118
*/
118119
public static final Option<Integer> PREPARED_STATEMENT_CACHE_QUERIES = Option.valueOf("preparedStatementCacheQueries");
119120

@@ -125,6 +126,7 @@ public final class PostgresqlConnectionFactoryProvider implements ConnectionFact
125126
/**
126127
* Returns a new {@link PostgresqlConnectionConfiguration.Builder} configured with the given {@link ConnectionFactoryOptions}.
127128
*
129+
* @param connectionFactoryOptions {@link ConnectionFactoryOptions} used to initialize the {@link PostgresqlConnectionConfiguration.Builder}.
128130
* @return a {@link PostgresqlConnectionConfiguration.Builder}
129131
* @since 0.9
130132
*/

‎src/main/java/io/r2dbc/postgresql/StatementCache.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ static StatementCache fromPreparedStatementCacheQueries(Client client, int prepa
3131
if (preparedStatementCacheQueries == 0) {
3232
return new DisabledStatementCache(client);
3333
}
34-
return new LimitedStatementCache(client, preparedStatementCacheQueries);
34+
return new BoundedStatementCache(client, preparedStatementCacheQueries);
3535
}
3636
}

‎src/test/java/io/r2dbc/postgresql/LimitedStatementCacheTest.java renamed to ‎src/test/java/io/r2dbc/postgresql/BoundedStatementCacheTest.java

+24-21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019 the original author or authors.
2+
* Copyright 2019-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,56 +38,57 @@
3838
import static io.r2dbc.postgresql.client.TestClient.NO_OP;
3939
import static io.r2dbc.postgresql.message.Format.FORMAT_BINARY;
4040
import static io.r2dbc.postgresql.util.TestByteBufAllocator.TEST;
41+
import static org.assertj.core.api.Assertions.assertThat;
4142
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
4243

43-
class LimitedStatementCacheTest {
44+
final class BoundedStatementCacheTest {
4445

4546
@Test
4647
void constructorInvalidLimit() {
47-
assertThatIllegalArgumentException().isThrownBy(() -> new LimitedStatementCache(NO_OP, -1))
48+
assertThatIllegalArgumentException().isThrownBy(() -> new BoundedStatementCache(NO_OP, -1))
4849
.withMessage("statement cache limit must be greater than zero");
49-
assertThatIllegalArgumentException().isThrownBy(() -> new LimitedStatementCache(NO_OP, 0))
50+
assertThatIllegalArgumentException().isThrownBy(() -> new BoundedStatementCache(NO_OP, 0))
5051
.withMessage("statement cache limit must be greater than zero");
5152
}
5253

5354
@Test
5455
void constructorNoClient() {
55-
assertThatIllegalArgumentException().isThrownBy(() -> new LimitedStatementCache(null, 2))
56+
assertThatIllegalArgumentException().isThrownBy(() -> new BoundedStatementCache(null, 2))
5657
.withMessage("client must not be null");
5758
}
5859

5960
@Test
6061
void getName() {
6162
// @formatter:off
6263
Client client = TestClient.builder()
63-
.expectRequest(new Parse("S_0", Collections.singletonList(100), "test-query"), Flush.INSTANCE)
64+
.expectRequest(new Parse("S_0", Collections.singletonList(100), "test-query-0"), Flush.INSTANCE)
6465
.thenRespond(ParseComplete.INSTANCE)
65-
.expectRequest(new Parse("S_1", Collections.singletonList(200), "test-query"), Flush.INSTANCE)
66+
.expectRequest(new Parse("S_1", Collections.singletonList(200), "test-query-1"), Flush.INSTANCE)
6667
.thenRespond(ParseComplete.INSTANCE)
6768
.expectRequest(new Close("S_0", ExecutionType.STATEMENT), Sync.INSTANCE)
6869
.thenRespond(CloseComplete.INSTANCE)
6970
.expectRequest(new Parse("S_2", Collections.singletonList(200), "test-query-2"), Flush.INSTANCE)
7071
.thenRespond(ParseComplete.INSTANCE)
7172
.expectRequest(new Close("S_2", ExecutionType.STATEMENT), Sync.INSTANCE)
7273
.thenRespond(CloseComplete.INSTANCE)
73-
.expectRequest(new Parse("S_0", Collections.singletonList(100), "test-query"), Flush.INSTANCE)
74+
.expectRequest(new Parse("S_3", Collections.singletonList(100), "test-query-0"), Flush.INSTANCE)
7475
.thenRespond(ParseComplete.INSTANCE)
7576
.build();
7677
// @formatter:on
7778

78-
LimitedStatementCache statementCache = new LimitedStatementCache(client, 2);
79+
BoundedStatementCache statementCache = new BoundedStatementCache(client, 2);
7980

80-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query")
81+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query-0")
8182
.as(StepVerifier::create)
8283
.expectNext("S_0")
8384
.verifyComplete();
8485

85-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(200)))), "test-query")
86+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(200)))), "test-query-0")
8687
.as(StepVerifier::create)
8788
.expectNext("S_0")
8889
.verifyComplete();
8990

90-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 200, Flux.just(TEST.buffer(2).writeShort(300)))), "test-query")
91+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 200, Flux.just(TEST.buffer(2).writeShort(300)))), "test-query-1")
9192
.as(StepVerifier::create)
9293
.expectNext("S_1")
9394
.verifyComplete();
@@ -97,20 +98,22 @@ void getName() {
9798
.expectNext("S_2")
9899
.verifyComplete();
99100

100-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 200, Flux.just(TEST.buffer(2).writeShort(300)))), "test-query")
101+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 200, Flux.just(TEST.buffer(2).writeShort(300)))), "test-query-1")
101102
.as(StepVerifier::create)
102103
.expectNext("S_1")
103104
.verifyComplete();
104105

105-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query")
106+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query-0")
106107
.as(StepVerifier::create)
107-
.expectNext("S_0")
108+
.expectNext("S_3")
108109
.verifyComplete();
109110

110-
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query")
111+
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(100)))), "test-query-0")
111112
.as(StepVerifier::create)
112-
.expectNext("S_0")
113+
.expectNext("S_3")
113114
.verifyComplete();
115+
116+
assertThat(statementCache.getCachedStatementNames()).hasSize(2).containsOnly("S_1", "S_3");
114117
}
115118

116119
@Test
@@ -122,7 +125,7 @@ void getNameErrorResponse() {
122125
.build();
123126
// @formatter:on
124127

125-
LimitedStatementCache statementCache = new LimitedStatementCache(client, 2);
128+
BoundedStatementCache statementCache = new BoundedStatementCache(client, 2);
126129

127130
statementCache.getName(new Binding(1).add(0, new Parameter(FORMAT_BINARY, 100, Flux.just(TEST.buffer(4).writeInt(200)))), "test-query")
128131
.as(StepVerifier::create)
@@ -131,14 +134,14 @@ void getNameErrorResponse() {
131134

132135
@Test
133136
void getNameNoBinding() {
134-
assertThatIllegalArgumentException().isThrownBy(() -> new LimitedStatementCache(NO_OP, 2).getName(null, "test-query"))
137+
assertThatIllegalArgumentException().isThrownBy(() -> new BoundedStatementCache(NO_OP, 2).getName(null, "test-query"))
135138
.withMessage("binding must not be null");
136139
}
137140

138141
@Test
139142
void getNameNoSql() {
140-
assertThatIllegalArgumentException().isThrownBy(() -> new LimitedStatementCache(NO_OP, 2).getName(new Binding(0), null))
143+
assertThatIllegalArgumentException().isThrownBy(() -> new BoundedStatementCache(NO_OP, 2).getName(new Binding(0), null))
141144
.withMessage("sql must not be null");
142145
}
143146

144-
}
147+
}

‎src/test/java/io/r2dbc/postgresql/DisabledStatementCacheTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019 the original author or authors.
2+
* Copyright 2019-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -110,4 +110,4 @@ void getNameNoSql() {
110110
.withMessage("sql must not be null");
111111
}
112112

113-
}
113+
}

‎src/test/java/io/r2dbc/postgresql/PostgresqlConnectionFactoryProviderTest.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017-2019 the original author or authors.
2+
* Copyright 2017-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -185,10 +185,10 @@ void providerShouldConsiderPreparedStatementCacheQueries() {
185185
.option(HOST, "test-host")
186186
.option(PASSWORD, "test-password")
187187
.option(USER, "test-user")
188-
.option(PREPARED_STATEMENT_CACHE_QUERIES, -1)
188+
.option(PREPARED_STATEMENT_CACHE_QUERIES, -2)
189189
.build());
190190

191-
assertThat(factory.getConfiguration().getPreparedStatementCacheQueries()).isEqualTo(-1);
191+
assertThat(factory.getConfiguration().getPreparedStatementCacheQueries()).isEqualTo(-2);
192192
}
193193

194194
@Test
@@ -198,10 +198,10 @@ void providerShouldConsiderPreparedStatementCacheQueriesWhenProvidedAsString() {
198198
.option(HOST, "test-host")
199199
.option(PASSWORD, "test-password")
200200
.option(USER, "test-user")
201-
.option(Option.valueOf("preparedStatementCacheQueries"), "-1")
201+
.option(Option.valueOf("preparedStatementCacheQueries"), "5")
202202
.build());
203203

204-
assertThat(factory.getConfiguration().getPreparedStatementCacheQueries()).isEqualTo(-1);
204+
assertThat(factory.getConfiguration().getPreparedStatementCacheQueries()).isEqualTo(5);
205205
}
206206

207207
@Test

0 commit comments

Comments
 (0)