Skip to content

Commit df95f5e

Browse files
committed
Refactored select command to route to all nodes per default
Removed the option to provide a route Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>
1 parent 3eca9c2 commit df95f5e

File tree

3 files changed

+15
-159
lines changed

3 files changed

+15
-159
lines changed

java/client/src/main/java/glide/api/GlideClusterClient.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static glide.api.models.commands.function.FunctionListOptions.LIBRARY_NAME_VALKEY_API;
4242
import static glide.api.models.commands.function.FunctionListOptions.WITH_CODE_VALKEY_API;
4343
import static glide.api.models.commands.function.FunctionLoadOptions.REPLACE;
44+
import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES;
4445
import static glide.utils.ArrayTransformUtils.castArray;
4546
import static glide.utils.ArrayTransformUtils.castMapOfArrays;
4647
import static glide.utils.ArrayTransformUtils.concatenateArrays;
@@ -471,21 +472,12 @@ public CompletableFuture<ClusterValue<GlideString>> echo(
471472
}
472473

473474
@Override
474-
public CompletableFuture<String> select(long index) {
475-
return commandManager.submitNewCommand(
476-
Select, new String[] {Long.toString(index)}, this::handleStringResponse);
477-
}
478-
479-
@Override
480-
public CompletableFuture<ClusterValue<String>> select(long index, @NonNull Route route) {
475+
public CompletableFuture<ClusterValue<String>> select(long index) {
481476
return commandManager.submitNewCommand(
482477
Select,
483478
new String[] {Long.toString(index)},
484-
route,
485-
response ->
486-
route instanceof SingleNodeRoute
487-
? ClusterValue.ofSingleValue(handleStringResponse(response))
488-
: ClusterValue.ofMultiValue(handleMapResponse(response)));
479+
ALL_NODES,
480+
response -> ClusterValue.ofMultiValue(handleMapResponse(response)));
489481
}
490482

491483
@Override

java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -250,39 +250,6 @@ public interface ConnectionManagementClusterCommands {
250250
*/
251251
CompletableFuture<ClusterValue<GlideString>> echo(GlideString message, Route route);
252252

253-
/**
254-
* Changes the currently selected database on cluster nodes.<br>
255-
* The command will be routed to all nodes.
256-
*
257-
* <p><b>WARNING:</b> This command is <b>NOT RECOMMENDED</b> for production use. Upon
258-
* reconnection, nodes will revert to the database_id specified in the client configuration
259-
* (default: 0), NOT the database selected via this command.
260-
*
261-
* <p><b>RECOMMENDED APPROACH:</b> Use the database_id parameter in client configuration instead:
262-
*
263-
* <pre>{@code
264-
* GlideClusterClient client = GlideClusterClient.createClient(
265-
* GlideClusterClientConfiguration.builder()
266-
* .address(NodeAddress.builder().host("localhost").port(6379).build())
267-
* .databaseId(5) // Recommended: persists across reconnections
268-
* .build()
269-
* ).get();
270-
* }</pre>
271-
*
272-
* <p><b>CLUSTER BEHAVIOR:</b> This command routes to all nodes by default to maintain consistency
273-
* across the cluster.
274-
*
275-
* @see <a href="https://valkey.io/commands/select/">valkey.io</a> for details.
276-
* @param index The index of the database to select.
277-
* @return A simple <code>OK</code> response.
278-
* @example
279-
* <pre>{@code
280-
* String response = clusterClient.select(0).get();
281-
* assert response.equals("OK");
282-
* }</pre>
283-
*/
284-
CompletableFuture<String> select(long index);
285-
286253
/**
287254
* Changes the currently selected database on cluster nodes.
288255
*
@@ -305,7 +272,7 @@ public interface ConnectionManagementClusterCommands {
305272
* @example
306273
* <pre>{@code
307274
* // Command sent to all nodes via ALL_NODES route, expecting a MultiValue result.
308-
* Map<String, String> responsePerNode = clusterClient.select(5, ALL_NODES).get().getMultiValue();
275+
* Map<String, String> responsePerNode = clusterClient.select(5).get().getMultiValue();
309276
* for(var response : responsePerNode.values()) {
310277
* assert response.equals("OK");
311278
* }
@@ -315,5 +282,5 @@ public interface ConnectionManagementClusterCommands {
315282
* assert response.equals("OK");
316283
* }</pre>
317284
*/
318-
CompletableFuture<ClusterValue<String>> select(long index, Route route);
285+
CompletableFuture<ClusterValue<String>> select(long index);
319286
}

java/integTest/src/test/java/glide/SharedCommandTests.java

Lines changed: 9 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static glide.api.models.commands.SetOptions.Expiry.Milliseconds;
1919
import static glide.api.models.commands.SortBaseOptions.OrderBy.ASC;
2020
import static glide.api.models.commands.SortBaseOptions.OrderBy.DESC;
21-
import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES;
2221
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
2322
import static org.junit.jupiter.api.Assertions.assertEquals;
2423
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -117,7 +116,6 @@
117116
import glide.api.models.commands.stream.StreamTrimOptions.MinId;
118117
import glide.api.models.configuration.ProtocolVersion;
119118
import glide.api.models.exceptions.RequestException;
120-
import glide.cluster.ValkeyCluster;
121119
import java.time.Instant;
122120
import java.util.ArrayList;
123121
import java.util.Arrays;
@@ -17620,54 +17618,6 @@ private static boolean isServerVersionAtLeast9_0() {
1762017618
return SERVER_VERSION.isGreaterThanOrEqualTo("9.0.0");
1762117619
}
1762217620

17623-
@SneakyThrows
17624-
private ValkeyCluster createDedicatedCluster(boolean clusterMode) {
17625-
return new ValkeyCluster(false, clusterMode, clusterMode ? 3 : 1, 0, null, null);
17626-
}
17627-
17628-
@SneakyThrows
17629-
@ParameterizedTest(autoCloseArguments = false)
17630-
@MethodSource("getClients")
17631-
public void select_command_standalone(BaseClient client) {
17632-
// Only test SELECT on standalone clients
17633-
if (client instanceof GlideClusterClient) {
17634-
return; // Skip cluster clients for this test
17635-
}
17636-
17637-
GlideClient standaloneClient = (GlideClient) client;
17638-
String key1 = UUID.randomUUID().toString();
17639-
String key2 = UUID.randomUUID().toString();
17640-
17641-
// Start in database 0, set a value
17642-
assertEquals(OK, standaloneClient.set(key1, "value_db0").get());
17643-
assertEquals("value_db0", standaloneClient.get(key1).get());
17644-
17645-
// Switch to database 1
17646-
assertEquals(OK, standaloneClient.select(1).get());
17647-
17648-
// Key from DB 0 should not exist in DB 1
17649-
assertEquals(null, standaloneClient.get(key1).get());
17650-
17651-
// Set a value in database 1
17652-
assertEquals(OK, standaloneClient.set(key2, "value_db1").get());
17653-
assertEquals("value_db1", standaloneClient.get(key2).get());
17654-
17655-
// Switch back to database 0
17656-
assertEquals(OK, standaloneClient.select(0).get());
17657-
17658-
// Original key should still exist in DB 0
17659-
assertEquals("value_db0", standaloneClient.get(key1).get());
17660-
17661-
// Key from DB 1 should not exist in DB 0
17662-
assertEquals(null, standaloneClient.get(key2).get());
17663-
17664-
// Clean up
17665-
assertEquals(1L, standaloneClient.del(new String[] {key1}).get());
17666-
assertEquals(OK, standaloneClient.select(1).get());
17667-
assertEquals(1L, standaloneClient.del(new String[] {key2}).get());
17668-
assertEquals(OK, standaloneClient.select(0).get()); // Reset to DB 0
17669-
}
17670-
1767117621
@SneakyThrows
1767217622
@ParameterizedTest(autoCloseArguments = false)
1767317623
@MethodSource("getClients")
@@ -17683,52 +17633,27 @@ public void select_command_cluster_basic_functionality(BaseClient client) {
1768317633
GlideClusterClient clusterClient = (GlideClusterClient) client;
1768417634
String key = UUID.randomUUID().toString();
1768517635

17686-
// Test that SELECT command returns OK
17687-
String selectResult = clusterClient.select(1).get();
17688-
assertEquals(OK, selectResult);
17636+
// Switch to database 1 with explicit ALL_NODES routing
17637+
ClusterValue<String> selectResult = clusterClient.select(1).get();
17638+
// When routing to ALL_NODES, we get a multi-value result
17639+
for (String result : selectResult.getMultiValue().values()) {
17640+
assertEquals(OK, result);
17641+
}
1768917642

1769017643
// Test that we can perform operations after SELECT
1769117644
assertEquals(OK, clusterClient.set(key, "test_value").get());
1769217645
assertEquals("test_value", clusterClient.get(key).get());
1769317646

17694-
// Switch back to database 0
17647+
// Switch to database 0 with explicit ALL_NODES routing
1769517648
selectResult = clusterClient.select(0).get();
17696-
assertEquals(OK, selectResult);
17697-
17698-
// Clean up
17699-
clusterClient.select(1).get();
17700-
clusterClient.del(new String[] {key}).get();
17701-
clusterClient.select(0).get(); // Reset to DB 0
17702-
}
17703-
17704-
@SneakyThrows
17705-
@ParameterizedTest(autoCloseArguments = false)
17706-
@MethodSource("getClients")
17707-
@EnabledIf("isServerVersionAtLeast9_0")
17708-
public void select_command_cluster_with_explicit_routing(BaseClient client) {
17709-
// Only test SELECT on cluster clients with Valkey 9.0+
17710-
if (!(client instanceof GlideClusterClient)) {
17711-
return; // Skip standalone clients for this test
17712-
}
17713-
17714-
assumeTrue(isServerVersionAtLeast9_0(), "Multi-DB cluster mode requires Valkey 9.0+");
17715-
17716-
GlideClusterClient clusterClient = (GlideClusterClient) client;
17717-
String key = UUID.randomUUID().toString();
17718-
17719-
// Switch to database 2 with explicit ALL_NODES routing
17720-
ClusterValue<String> selectResult = clusterClient.select(2, ALL_NODES).get();
1772117649
// When routing to ALL_NODES, we get a multi-value result
1772217650
for (String result : selectResult.getMultiValue().values()) {
1772317651
assertEquals(OK, result);
1772417652
}
1772517653

17726-
// Verify we're in database 2 by setting and getting a value
17727-
assertEquals(OK, clusterClient.set(key, "value_db2").get());
17728-
assertEquals("value_db2", clusterClient.get(key).get());
17729-
1773017654
// Clean up
17731-
assertEquals(1L, clusterClient.del(new String[] {key}).get());
17655+
clusterClient.select(1).get();
17656+
clusterClient.del(new String[] {key}).get();
1773217657
clusterClient.select(0).get(); // Reset to DB 0
1773317658
}
1773417659

@@ -17755,32 +17680,4 @@ public void select_command_invalid_database_standalone(BaseClient client, int in
1775517680
exception.getMessage().toLowerCase().contains("invalid")
1775617681
|| exception.getMessage().toLowerCase().contains("out of range"));
1775717682
}
17758-
17759-
@SneakyThrows
17760-
@ParameterizedTest(autoCloseArguments = false)
17761-
@MethodSource("getClients")
17762-
public void select_command_cluster_on_older_server(BaseClient client) {
17763-
// Only test SELECT on cluster clients on older servers
17764-
if (!(client instanceof GlideClusterClient)) {
17765-
return; // Skip standalone clients for this test
17766-
}
17767-
17768-
if (isServerVersionAtLeast9_0()) {
17769-
assumeTrue(false, "Skipping test on Valkey 9.0+ where multi-DB cluster is supported");
17770-
}
17771-
17772-
GlideClusterClient clusterClient = (GlideClusterClient) client;
17773-
17774-
ExecutionException exception =
17775-
assertThrows(
17776-
ExecutionException.class,
17777-
() -> {
17778-
clusterClient.select(1).get();
17779-
});
17780-
17781-
assertTrue(
17782-
exception.getMessage().toLowerCase().contains("select")
17783-
|| exception.getMessage().toLowerCase().contains("cluster")
17784-
|| exception.getMessage().toLowerCase().contains("not allowed"));
17785-
}
1778617683
}

0 commit comments

Comments
 (0)