Skip to content

Commit 0f6a152

Browse files
mp911demp911de
committed
Polishing.
Reorder methods. Move rewriteConfig directly implemented on connection to DefaultedRedisConnection respective DefaultedRedisClusterConnection. Add overload to run config rewrite on individual cluster nodes. Add tests. Add since tags and update Javadoc. See #1992 Original pull request: #2012.
1 parent 2d56eaf commit 0f6a152

13 files changed

+134
-34
lines changed

Diff for: ‎src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,15 @@ public void resetConfigStats() {
878878
delegate.resetConfigStats();
879879
}
880880

881+
/*
882+
* (non-Javadoc)
883+
* @see org.springframework.data.redis.connection.RedisServerCommands#rewriteConfig()
884+
*/
885+
@Override
886+
public void rewriteConfig() {
887+
delegate.rewriteConfig();
888+
}
889+
881890
/*
882891
* (non-Javadoc)
883892
* @see org.springframework.data.redis.connection.RedisListCommands#rPop(byte[])
@@ -3731,11 +3740,6 @@ public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable Migrate
37313740
delegate.migrate(key, target, dbIndex, option, timeout);
37323741
}
37333742

3734-
@Override
3735-
public void rewriteConfig() {
3736-
delegate.rewriteConfig();
3737-
}
3738-
37393743
/*
37403744
* (non-Javadoc)
37413745
* @see org.springframework.data.redis.connection.StringRedisConnection#xAck(java.lang.String, java.lang.String, RecordId[])

Diff for: ‎src/main/java/org/springframework/data/redis/connection/DefaultedRedisClusterConnection.java

+7
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ default void resetConfigStats(RedisClusterNode node) {
122122
serverCommands().resetConfigStats(node);
123123
}
124124

125+
/** @deprecated in favor of {@link RedisConnection#serverCommands()}. */
126+
@Override
127+
@Deprecated
128+
default void rewriteConfig(RedisClusterNode node) {
129+
serverCommands().rewriteConfig(node);
130+
}
131+
125132
/** @deprecated in favor of {@link RedisConnection#serverCommands()}. */
126133
@Override
127134
@Deprecated

Diff for: ‎src/main/java/org/springframework/data/redis/connection/DefaultedRedisConnection.java

+7
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,13 @@ default void resetConfigStats() {
14301430
serverCommands().resetConfigStats();
14311431
}
14321432

1433+
/** @deprecated in favor of {@link RedisConnection#serverCommands()}. */
1434+
@Override
1435+
@Deprecated
1436+
default void rewriteConfig() {
1437+
serverCommands().rewriteConfig();
1438+
}
1439+
14331440
/** @deprecated in favor of {@link RedisConnection#serverCommands()}. */
14341441
@Override
14351442
@Deprecated

Diff for: ‎src/main/java/org/springframework/data/redis/connection/RedisClusterServerCommands.java

+7
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ public interface RedisClusterServerCommands extends RedisServerCommands {
114114
*/
115115
void resetConfigStats(RedisClusterNode node);
116116

117+
/**
118+
* @param node must not be {@literal null}.
119+
* @see RedisServerCommands#rewriteConfig()
120+
* @since 2.5
121+
*/
122+
void rewriteConfig(RedisClusterNode node);
123+
117124
/**
118125
* @param node must not be {@literal null}.
119126
* @return

Diff for: ‎src/main/java/org/springframework/data/redis/connection/RedisServerCommands.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,14 @@ default void bgWriteAof() {
174174
*/
175175
void resetConfigStats();
176176

177+
/**
178+
* Rewrites the {@code redis.conf} file.
179+
*
180+
* @since 2.5
181+
* @see <a href="https://redis.io/commands/config-rewrite">Redis Documentation: CONFIG REWRITE</a>
182+
*/
183+
void rewriteConfig();
184+
177185
/**
178186
* Request server timestamp using {@code TIME} command in {@link TimeUnit#MILLISECONDS}.
179187
*
@@ -281,8 +289,4 @@ default Long time() {
281289
*/
282290
void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable MigrateOption option, long timeout);
283291

284-
/**
285-
* Rewrites the redis.conf file.
286-
*/
287-
void rewriteConfig();
288292
}

Diff for: ‎src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterServerCommands.java

+19-6
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,16 @@ public void resetConfigStats() {
381381
.executeCommandOnAllNodes((JedisClusterCommandCallback<String>) BinaryJedis::configResetStat);
382382
}
383383

384+
/*
385+
* (non-Javadoc)
386+
* @see org.springframework.data.redis.connection.RedisServerCommands#rewriteConfig()
387+
*/
388+
@Override
389+
public void rewriteConfig() {
390+
connection.getClusterCommandExecutor()
391+
.executeCommandOnAllNodes((JedisClusterCommandCallback<String>) BinaryJedis::configRewrite);
392+
}
393+
384394
/*
385395
* (non-Javadoc)
386396
* @see org.springframework.data.redis.connection.RedisClusterServerCommands#resetConfigStats(org.springframework.data.redis.connection.RedisClusterNode)
@@ -390,6 +400,15 @@ public void resetConfigStats(RedisClusterNode node) {
390400
executeCommandOnSingleNode(BinaryJedis::configResetStat, node);
391401
}
392402

403+
/*
404+
* (non-Javadoc)
405+
* @see org.springframework.data.redis.connection.RedisClusterServerCommands#rewriteConfig(org.springframework.data.redis.connection.RedisClusterNode)
406+
*/
407+
@Override
408+
public void rewriteConfig(RedisClusterNode node) {
409+
executeCommandOnSingleNode(BinaryJedis::configRewrite, node);
410+
}
411+
393412
/*
394413
* (non-Javadoc)
395414
* @see org.springframework.data.redis.connection.RedisServerCommands#time(TimeUnit)
@@ -520,12 +539,6 @@ public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable Migrate
520539
node);
521540
}
522541

523-
@Override
524-
public void rewriteConfig() {
525-
connection.getClusterCommandExecutor()
526-
.executeCommandOnAllNodes((JedisClusterCommandCallback<String>) client -> client.configRewrite());
527-
}
528-
529542
private Long convertListOfStringToTime(List<String> serverTimeInformation, TimeUnit timeUnit) {
530543

531544
Assert.notEmpty(serverTimeInformation, "Received invalid result from server. Expected 2 items in collection.");

Diff for: ‎src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java

-4
Original file line numberDiff line numberDiff line change
@@ -806,8 +806,4 @@ private void doWithJedis(Consumer<Jedis> callback) {
806806
}
807807
}
808808

809-
@Override
810-
public void rewriteConfig() {
811-
serverCommands().rewriteConfig();
812-
}
813809
}

Diff for: ‎src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,15 @@ public void resetConfigStats() {
189189
connection.invokeStatus().just(BinaryJedis::configResetStat, MultiKeyPipelineBase::configResetStat);
190190
}
191191

192+
/*
193+
* (non-Javadoc)
194+
* @see org.springframework.data.redis.connection.RedisServerCommands#resetConfigStats()
195+
*/
196+
@Override
197+
public void rewriteConfig() {
198+
connection.invokeStatus().just(Jedis::configRewrite);
199+
}
200+
192201
/*
193202
* (non-Javadoc)
194203
* @see org.springframework.data.redis.connection.RedisServerCommands#time(TimeUnit)
@@ -317,11 +326,6 @@ public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable Migrate
317326
target.getPort(), key, dbIndex, timeoutToUse);
318327
}
319328

320-
@Override
321-
public void rewriteConfig() {
322-
connection.invokeStatus().just(Jedis::configRewrite);
323-
}
324-
325329
private boolean isPipelined() {
326330
return connection.isPipelined();
327331
}

Diff for: ‎src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterServerCommands.java

+18
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,24 @@ public void resetConfigStats(RedisClusterNode node) {
307307
executeCommandOnSingleNode(RedisServerCommands::configResetstat, node);
308308
}
309309

310+
/*
311+
* (non-Javadoc)
312+
* @see org.springframework.data.redis.connection.lettuce.LettuceServerCommands#rewriteConfig()
313+
*/
314+
@Override
315+
public void rewriteConfig() {
316+
executeCommandOnAllNodes(RedisServerCommands::configRewrite);
317+
}
318+
319+
/*
320+
* (non-Javadoc)
321+
* @see org.springframework.data.redis.connection.RedisClusterServerCommands#rewriteConfig(org.springframework.data.redis.connection.RedisClusterNode)
322+
*/
323+
@Override
324+
public void rewriteConfig(RedisClusterNode node) {
325+
executeCommandOnSingleNode(RedisServerCommands::configRewrite, node);
326+
}
327+
310328
/*
311329
* (non-Javadoc)
312330
* @see org.springframework.data.redis.connection.RedisClusterServerCommands#time(org.springframework.data.redis.connection.RedisClusterNode)

Diff for: ‎src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java

-5
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,6 @@ <T, R> LettuceResult<T, R> newLettuceStatusResult(Future<T> resultHolder) {
137137
return LettuceResultBuilder.<T, R> forResponse(resultHolder).buildStatusResult();
138138
}
139139

140-
@Override
141-
public void rewriteConfig() {
142-
serverCommands().rewriteConfig();
143-
}
144-
145140
private class LettuceTransactionResultConverter<T> extends TransactionResultConverter<T> {
146141
public LettuceTransactionResultConverter(Queue<FutureResult<T>> txResults,
147142
Converter<Exception, DataAccessException> exceptionConverter) {

Diff for: ‎src/main/java/org/springframework/data/redis/connection/lettuce/LettuceServerCommands.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ public void resetConfigStats() {
199199
connection.invokeStatus().just(RedisServerAsyncCommands::configResetstat);
200200
}
201201

202+
/*
203+
* (non-Javadoc)
204+
* @see org.springframework.data.redis.connection.RedisServerCommands#resetConfigStats()
205+
*/
206+
@Override
207+
public void rewriteConfig() {
208+
connection.invokeStatus().just(RedisServerAsyncCommands::configRewrite);
209+
}
210+
202211
/*
203212
* (non-Javadoc)
204213
* @see org.springframework.data.redis.connection.RedisServerCommands#time(TimeUnit)
@@ -304,11 +313,6 @@ public void migrate(byte[] key, RedisNode target, int dbIndex, @Nullable Migrate
304313
connection.invoke().just(RedisKeyAsyncCommands::migrate, target.getHost(), target.getPort(), key, dbIndex, timeout);
305314
}
306315

307-
@Override
308-
public void rewriteConfig() {
309-
connection.invoke().just(RedisServerAsyncCommands::configRewrite);
310-
}
311-
312316
public RedisClusterCommands<byte[], byte[]> getConnection() {
313317
return connection.getConnection();
314318
}

Diff for: ‎src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java

+21
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,27 @@ void resetConfigStatsShouldBeExecutedOnSingleNodeCorrectly() {
333333
verify(con3Mock, never()).configResetStat();
334334
}
335335

336+
@Test // GH-1992
337+
void rewriteConfigShouldBeExecutedOnAllNodes() {
338+
339+
connection.rewriteConfig();
340+
341+
verify(con1Mock, times(1)).configRewrite();
342+
verify(con2Mock, times(1)).configRewrite();
343+
verify(con3Mock, times(1)).configRewrite();
344+
}
345+
346+
@Test // GH-1992
347+
void rewriteConfigShouldBeExecutedOnSingleNodeCorrectly() {
348+
349+
connection.rewriteConfig(CLUSTER_NODE_2);
350+
351+
verify(con2Mock, times(1)).configRewrite();
352+
verify(con2Mock, atLeast(1)).close();
353+
verify(con1Mock, never()).configRewrite();
354+
verify(con3Mock, never()).configRewrite();
355+
}
356+
336357
@Test // DATAREDIS-315
337358
void clusterTopologyProviderShouldCollectErrorsWhenLoadingNodes() {
338359

Diff for: ‎src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionUnitTests.java

+20
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,26 @@ void resetConfigStatsShouldBeExecutedOnSingleNodeCorrectly() {
357357
verify(clusterConnection1Mock, never()).configResetstat();
358358
}
359359

360+
@Test // GH-1992
361+
void rewriteConfigShouldBeExecutedOnAllNodes() {
362+
363+
connection.rewriteConfig();
364+
365+
verify(clusterConnection1Mock, times(1)).configRewrite();
366+
verify(clusterConnection2Mock, times(1)).configRewrite();
367+
verify(clusterConnection3Mock, times(1)).configRewrite();
368+
}
369+
370+
@Test // GH-1992
371+
void rewriteConfigShouldBeExecutedOnSingleNodeCorrectly() {
372+
373+
connection.rewriteConfig(CLUSTER_NODE_2);
374+
375+
verify(clusterConnection2Mock, times(1)).configRewrite();
376+
verify(clusterConnection1Mock, never()).configRewrite();
377+
verify(clusterConnection1Mock, never()).configRewrite();
378+
}
379+
360380
@Test // DATAREDIS-731, DATAREDIS-545
361381
void shouldExecuteOnSharedConnection() {
362382

0 commit comments

Comments
 (0)