Skip to content

Commit 1ae7219

Browse files
atakavciatakavci
committed
- replace use of reflection with helper methods
- fix failing tests due to method name change
1 parent b8d4e87 commit 1ae7219

File tree

5 files changed

+73
-197
lines changed

5 files changed

+73
-197
lines changed

‎src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ private void addClusterInternal(MultiClusterClientConfig multiClusterClientConfi
311311
* Handles health status changes for clusters. This method is called by the health status manager when the health
312312
* status of a cluster changes.
313313
*/
314-
private void onHealthStatusChange(HealthStatusChangeEvent eventArgs) {
314+
@VisibleForTesting
315+
void onHealthStatusChange(HealthStatusChangeEvent eventArgs) {
315316
Endpoint endpoint = eventArgs.getEndpoint();
316317
HealthStatus newStatus = eventArgs.getNewStatus();
317318
log.debug("Health status changed for {} from {} to {}", endpoint, eventArgs.getOldStatus(), newStatus);
@@ -382,7 +383,8 @@ private Cluster waitForInitialHealthyCluster() {
382383
/**
383384
* Periodic failback checker - runs at configured intervals to check for failback opportunities
384385
*/
385-
private void periodicFailbackCheck() {
386+
@VisibleForTesting
387+
void periodicFailbackCheck() {
386388
try {
387389
// Find the best candidate cluster for failback
388390
Cluster bestCandidate = null;

‎src/test/java/redis/clients/jedis/mcf/FailbackMechanismIntegrationTest.java

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import redis.clients.jedis.JedisClientConfig;
1717
import redis.clients.jedis.MultiClusterClientConfig;
1818
import redis.clients.jedis.providers.MultiClusterPooledConnectionProvider;
19-
import java.lang.reflect.Method;
19+
import redis.clients.jedis.providers.MultiClusterPooledConnectionProviderHelper;
2020

2121
@ExtendWith(MockitoExtension.class)
2222
class FailbackMechanismIntegrationTest {
@@ -43,23 +43,6 @@ private MockedConstruction<ConnectionPool> mockPool() {
4343
});
4444
}
4545

46-
/**
47-
* Helper method to trigger health status changes using reflection
48-
*/
49-
private void triggerHealthStatusChange(MultiClusterPooledConnectionProvider provider, HostAndPort endpoint,
50-
HealthStatus oldStatus, HealthStatus newStatus) {
51-
try {
52-
Method handleHealthStatusChange = MultiClusterPooledConnectionProvider.class
53-
.getDeclaredMethod("handleHealthStatusChange", HealthStatusChangeEvent.class);
54-
handleHealthStatusChange.setAccessible(true);
55-
56-
HealthStatusChangeEvent event = new HealthStatusChangeEvent(endpoint, oldStatus, newStatus);
57-
handleHealthStatusChange.invoke(provider, event);
58-
} catch (Exception e) {
59-
throw new RuntimeException("Failed to trigger health status change", e);
60-
}
61-
}
62-
6346
@Test
6447
void testFailbackDisabledDoesNotPerformFailback() throws InterruptedException {
6548
try (MockedConstruction<ConnectionPool> mockedPool = mockPool()) {
@@ -81,13 +64,13 @@ void testFailbackDisabledDoesNotPerformFailback() throws InterruptedException {
8164
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
8265

8366
// Make cluster2 unhealthy to force failover to cluster1
84-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
67+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
8568

8669
// Should now be on cluster1 (only healthy option)
8770
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
8871

8972
// Make cluster2 healthy again (higher weight - would normally trigger failback)
90-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
73+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
9174

9275
// Wait longer than failback interval
9376
Thread.sleep(200);
@@ -121,13 +104,13 @@ void testFailbackToHigherWeightCluster() throws InterruptedException {
121104
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
122105

123106
// Make cluster1 unhealthy to force failover to cluster2
124-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
107+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
125108

126109
// Should now be on cluster2 (lower weight, but only healthy option)
127110
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
128111

129112
// Make cluster1 healthy again
130-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
113+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
131114

132115
// Wait for failback check interval + some buffer
133116
Thread.sleep(250);
@@ -163,14 +146,14 @@ void testNoFailbackToLowerWeightCluster() throws InterruptedException {
163146
assertEquals(provider.getCluster(endpoint3), provider.getCluster());
164147

165148
// Make cluster3 unhealthy to force failover to cluster2 (medium weight)
166-
triggerHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
149+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
167150

168151
// Should now be on cluster2 (highest weight among healthy clusters)
169152
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
170153

171154
// Make cluster1 (lowest weight) healthy - this should NOT trigger failback
172155
// since we don't failback to lower weight clusters
173-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
156+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
174157

175158
// Wait for failback check interval
176159
Thread.sleep(250);
@@ -199,13 +182,13 @@ void testFailbackToHigherWeightClusterImmediately() throws InterruptedException
199182
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
200183

201184
// Make cluster1 unhealthy to force failover to cluster2
202-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
185+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
203186

204187
// Should now be on cluster2 (only healthy option)
205188
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
206189

207190
// Make cluster1 healthy again
208-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
191+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
209192

210193
// Wait for failback check
211194
Thread.sleep(150);
@@ -234,19 +217,19 @@ void testUnhealthyClusterCancelsFailback() throws InterruptedException {
234217
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
235218

236219
// Make cluster1 unhealthy to force failover to cluster2
237-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
220+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
238221

239222
// Should now be on cluster2 (only healthy option)
240223
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
241224

242225
// Make cluster1 healthy again (should trigger failback attempt)
243-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
226+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
244227

245228
// Wait a bit
246229
Thread.sleep(100);
247230

248231
// Make cluster1 unhealthy again before failback completes
249-
triggerHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
232+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint1, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
250233

251234
// Wait past the original failback interval
252235
Thread.sleep(150);
@@ -279,13 +262,13 @@ void testMultipleClusterFailbackPriority() throws InterruptedException {
279262
assertEquals(provider.getCluster(endpoint3), provider.getCluster());
280263

281264
// Make cluster3 unhealthy to force failover to cluster2 (next highest weight)
282-
triggerHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
265+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
283266

284267
// Should now be on cluster2 (highest weight among healthy clusters)
285268
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
286269

287270
// Make cluster3 healthy again
288-
triggerHealthStatusChange(provider, endpoint3, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
271+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint3, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
289272

290273
// Wait for failback
291274
Thread.sleep(200);
@@ -315,7 +298,7 @@ void testGracePeriodDisablesClusterOnUnhealthy() throws InterruptedException {
315298
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
316299

317300
// Now make cluster2 unhealthy - it should be disabled for grace period
318-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
301+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
319302

320303
// Should failover to cluster1
321304
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
@@ -346,7 +329,7 @@ void testGracePeriodReEnablesClusterAfterPeriod() throws InterruptedException {
346329
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
347330

348331
// Make cluster2 unhealthy to start grace period and force failover
349-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
332+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
350333

351334
// Should failover to cluster1
352335
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
@@ -355,7 +338,7 @@ void testGracePeriodReEnablesClusterAfterPeriod() throws InterruptedException {
355338
assertTrue(provider.getCluster(endpoint2).isInGracePeriod());
356339

357340
// Make cluster2 healthy again while it's still in grace period
358-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
341+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
359342

360343
// Should still be on cluster1 because cluster2 is in grace period
361344
assertEquals(provider.getCluster(endpoint1), provider.getCluster());

‎src/test/java/redis/clients/jedis/mcf/PeriodicFailbackTest.java

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@
33
import static org.junit.jupiter.api.Assertions.*;
44
import static org.mockito.Mockito.*;
55

6-
import java.lang.reflect.Method;
7-
86
import org.junit.jupiter.api.BeforeEach;
97
import org.junit.jupiter.api.Test;
108
import org.junit.jupiter.api.extension.ExtendWith;
11-
import org.mockito.Mock;
129
import org.mockito.MockedConstruction;
1310
import org.mockito.junit.jupiter.MockitoExtension;
1411

@@ -19,6 +16,7 @@
1916
import redis.clients.jedis.JedisClientConfig;
2017
import redis.clients.jedis.MultiClusterClientConfig;
2118
import redis.clients.jedis.providers.MultiClusterPooledConnectionProvider;
19+
import redis.clients.jedis.providers.MultiClusterPooledConnectionProviderHelper;
2220

2321
@ExtendWith(MockitoExtension.class)
2422
class PeriodicFailbackTest {
@@ -43,38 +41,6 @@ private MockedConstruction<ConnectionPool> mockPool() {
4341
});
4442
}
4543

46-
/**
47-
* Helper method to trigger health status changes using reflection
48-
*/
49-
private void triggerHealthStatusChange(MultiClusterPooledConnectionProvider provider, HostAndPort endpoint,
50-
HealthStatus oldStatus, HealthStatus newStatus) {
51-
try {
52-
Method handleHealthStatusChange = MultiClusterPooledConnectionProvider.class
53-
.getDeclaredMethod("handleHealthStatusChange", HealthStatusChangeEvent.class);
54-
handleHealthStatusChange.setAccessible(true);
55-
56-
HealthStatusChangeEvent event = new HealthStatusChangeEvent(endpoint, oldStatus, newStatus);
57-
handleHealthStatusChange.invoke(provider, event);
58-
} catch (Exception e) {
59-
throw new RuntimeException("Failed to trigger health status change", e);
60-
}
61-
}
62-
63-
/**
64-
* Helper method to trigger periodic failback check using reflection
65-
*/
66-
private void triggerPeriodicFailbackCheck(MultiClusterPooledConnectionProvider provider) {
67-
try {
68-
Method periodicFailbackCheckMethod = MultiClusterPooledConnectionProvider.class
69-
.getDeclaredMethod("periodicFailbackCheck");
70-
periodicFailbackCheckMethod.setAccessible(true);
71-
72-
periodicFailbackCheckMethod.invoke(provider);
73-
} catch (Exception e) {
74-
throw new RuntimeException("Failed to trigger periodic failback check", e);
75-
}
76-
}
77-
7844
@Test
7945
void testPeriodicFailbackCheckWithDisabledCluster() throws InterruptedException {
8046
try (MockedConstruction<ConnectionPool> mockedPool = mockPool()) {
@@ -100,7 +66,7 @@ void testPeriodicFailbackCheckWithDisabledCluster() throws InterruptedException
10066
provider.iterateActiveCluster();
10167

10268
// Manually trigger periodic check
103-
triggerPeriodicFailbackCheck(provider);
69+
MultiClusterPooledConnectionProviderHelper.periodicFailbackCheck(provider);
10470

10571
// Should still be on cluster1 (cluster2 is in grace period)
10672
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
@@ -126,7 +92,7 @@ void testPeriodicFailbackCheckWithHealthyCluster() throws InterruptedException {
12692
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
12793

12894
// Make cluster2 unhealthy to force failover to cluster1
129-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
95+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
13096

13197
// Should now be on cluster1 (cluster2 is in grace period)
13298
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
@@ -135,17 +101,17 @@ void testPeriodicFailbackCheckWithHealthyCluster() throws InterruptedException {
135101
assertTrue(provider.getCluster(endpoint2).isInGracePeriod());
136102

137103
// Make cluster2 healthy again (but it's still in grace period)
138-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
104+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
139105

140106
// Trigger periodic check immediately - should still be on cluster1
141-
triggerPeriodicFailbackCheck(provider);
107+
MultiClusterPooledConnectionProviderHelper.periodicFailbackCheck(provider);
142108
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
143109

144110
// Wait for grace period to expire
145111
Thread.sleep(150);
146112

147113
// Trigger periodic check after grace period expires
148-
triggerPeriodicFailbackCheck(provider);
114+
MultiClusterPooledConnectionProviderHelper.periodicFailbackCheck(provider);
149115

150116
// Should have failed back to cluster2 (higher weight, grace period expired)
151117
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
@@ -171,19 +137,19 @@ void testPeriodicFailbackCheckWithFailbackDisabled() throws InterruptedException
171137
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
172138

173139
// Make cluster2 unhealthy to force failover to cluster1
174-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
140+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
175141

176142
// Should now be on cluster1
177143
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
178144

179145
// Make cluster2 healthy again
180-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
146+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
181147

182148
// Wait for stability period
183149
Thread.sleep(100);
184150

185151
// Trigger periodic check
186-
triggerPeriodicFailbackCheck(provider);
152+
MultiClusterPooledConnectionProviderHelper.periodicFailbackCheck(provider);
187153

188154
// Should still be on cluster1 (failback disabled)
189155
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
@@ -215,26 +181,26 @@ void testPeriodicFailbackCheckSelectsHighestWeightCluster() throws InterruptedEx
215181
assertEquals(provider.getCluster(endpoint3), provider.getCluster());
216182

217183
// Make cluster3 unhealthy to force failover to cluster2 (next highest weight)
218-
triggerHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
184+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint3, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
219185

220186
// Should now be on cluster2 (weight 2.0f, higher than cluster1's 1.0f)
221187
assertEquals(provider.getCluster(endpoint2), provider.getCluster());
222188

223189
// Make cluster2 unhealthy to force failover to cluster1
224-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
190+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.HEALTHY, HealthStatus.UNHEALTHY);
225191

226192
// Should now be on cluster1 (only healthy cluster left)
227193
assertEquals(provider.getCluster(endpoint1), provider.getCluster());
228194

229195
// Make cluster2 and cluster3 healthy again
230-
triggerHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
231-
triggerHealthStatusChange(provider, endpoint3, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
196+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint2, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
197+
MultiClusterPooledConnectionProviderHelper.onHealthStatusChange(provider, endpoint3, HealthStatus.UNHEALTHY, HealthStatus.HEALTHY);
232198

233199
// Wait for grace period to expire
234200
Thread.sleep(150);
235201

236202
// Trigger periodic check
237-
triggerPeriodicFailbackCheck(provider);
203+
MultiClusterPooledConnectionProviderHelper.periodicFailbackCheck(provider);
238204

239205
// Should have failed back to cluster3 (highest weight, grace period expired)
240206
assertEquals(provider.getCluster(endpoint3), provider.getCluster());

0 commit comments

Comments
 (0)