From 0c0eb8a3a532bd41d247d9bd7a61c0d6522fee47 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 12 Jun 2024 17:22:05 -0700 Subject: [PATCH 1/4] Java: Add `FUNCTION KILL` command. (#328) * Add `FUNCTION KILL` command. * Frantic and rampant code clean up. * More tests for the god of the tests! * Address PR comments. * Typo fix. * Fixing the tests s02ep06. * Fixing the tests s03ep01. * Address PR comments. Co-authored-by: aaron-congo Signed-off-by: Yury-Fridlyand --- glide-core/src/protobuf/redis_request.proto | 1 + glide-core/src/request_type.rs | 3 + .../src/main/java/glide/api/RedisClient.java | 6 + .../java/glide/api/RedisClusterClient.java | 12 + .../ScriptingAndFunctionsClusterCommands.java | 33 ++ .../ScriptingAndFunctionsCommands.java | 15 + .../glide/api/models/BaseTransaction.java | 15 + .../test/java/glide/api/RedisClientTest.java | 22 ++ .../glide/api/RedisClusterClientTest.java | 43 +++ .../glide/api/models/TransactionTests.java | 4 + .../src/test/java/glide/TestUtilities.java | 32 ++ .../cluster/ClusterTransactionTests.java | 20 + .../test/java/glide/cluster/CommandTests.java | 341 +++++++++++++++++- .../java/glide/standalone/CommandTests.java | 150 +++++++- .../glide/standalone/TransactionTests.java | 30 +- 15 files changed, 701 insertions(+), 26 deletions(-) diff --git a/glide-core/src/protobuf/redis_request.proto b/glide-core/src/protobuf/redis_request.proto index 1b6bceecfe..d6bb4d8ff7 100644 --- a/glide-core/src/protobuf/redis_request.proto +++ b/glide-core/src/protobuf/redis_request.proto @@ -200,6 +200,7 @@ enum RequestType { BLMPop = 158; XLen = 159; Sort = 160; + FunctionKill = 161; LSet = 165; XDel = 166; XRange = 167; diff --git a/glide-core/src/request_type.rs b/glide-core/src/request_type.rs index 7ae5b39293..5246489024 100644 --- a/glide-core/src/request_type.rs +++ b/glide-core/src/request_type.rs @@ -170,6 +170,7 @@ pub enum RequestType { BLMPop = 158, XLen = 159, Sort = 160, + FunctionKill = 161, LSet = 165, XDel = 166, XRange = 167, @@ -356,6 +357,7 @@ impl From<::protobuf::EnumOrUnknown> for RequestType { ProtobufRequestType::ExpireTime => RequestType::ExpireTime, ProtobufRequestType::PExpireTime => RequestType::PExpireTime, ProtobufRequestType::XLen => RequestType::XLen, + ProtobufRequestType::FunctionKill => RequestType::FunctionKill, ProtobufRequestType::LSet => RequestType::LSet, ProtobufRequestType::XDel => RequestType::XDel, ProtobufRequestType::XRange => RequestType::XRange, @@ -539,6 +541,7 @@ impl RequestType { RequestType::ExpireTime => Some(cmd("EXPIRETIME")), RequestType::PExpireTime => Some(cmd("PEXPIRETIME")), RequestType::XLen => Some(cmd("XLEN")), + RequestType::FunctionKill => Some(get_two_word_command("FUNCTION", "KILL")), RequestType::LSet => Some(cmd("LSET")), RequestType::XDel => Some(cmd("XDEL")), RequestType::XRange => Some(cmd("XRANGE")), diff --git a/java/client/src/main/java/glide/api/RedisClient.java b/java/client/src/main/java/glide/api/RedisClient.java index 4f675f7e11..4afb9b03e8 100644 --- a/java/client/src/main/java/glide/api/RedisClient.java +++ b/java/client/src/main/java/glide/api/RedisClient.java @@ -20,6 +20,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.Info; @@ -277,4 +278,9 @@ public CompletableFuture copy( } return commandManager.submitNewCommand(Copy, arguments, this::handleBooleanResponse); } + + @Override + public CompletableFuture functionKill() { + return commandManager.submitNewCommand(FunctionKill, new String[0], this::handleStringResponse); + } } diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 400e407d85..4520c15164 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -22,6 +22,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.Info; @@ -575,4 +576,15 @@ public CompletableFuture> fcall( ? ClusterValue.ofSingleValue(handleObjectOrNullResponse(response)) : ClusterValue.ofMultiValue(handleMapResponse(response))); } + + @Override + public CompletableFuture functionKill() { + return commandManager.submitNewCommand(FunctionKill, new String[0], this::handleStringResponse); + } + + @Override + public CompletableFuture functionKill(@NonNull Route route) { + return commandManager.submitNewCommand( + FunctionKill, new String[0], route, this::handleStringResponse); + } } diff --git a/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsClusterCommands.java b/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsClusterCommands.java index bb9eabf156..52d9119cbe 100644 --- a/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsClusterCommands.java +++ b/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsClusterCommands.java @@ -339,4 +339,37 @@ CompletableFuture[]>> functionList( * } */ CompletableFuture> fcall(String function, String[] arguments, Route route); + + /** + * Kills a function that is currently executing.
+ * FUNCTION KILL terminates read-only functions only.
+ * The command will be routed to all primary nodes. + * + * @since Redis 7.0 and above. + * @see redis.io for details. + * @return OK if function is terminated. Otherwise, throws an error. + * @example + * 
{@code
+     * String response = client.functionKill().get();
+     * assert response.equals("OK");
+     * }
+ */ + CompletableFuture functionKill(); + + /** + * Kills a function that is currently executing.
+ * FUNCTION KILL terminates read-only functions only. + * + * @since Redis 7.0 and above. + * @see redis.io for details. + * @param route Specifies the routing configuration for the command. The client will route the + * command to the nodes defined by route. + * @return OK if function is terminated. Otherwise, throws an error. + * @example + * 
{@code
+     * String response = client.functionKill(RANDOM).get();
+     * assert response.equals("OK");
+     * }
+ */ + CompletableFuture functionKill(Route route); } diff --git a/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsCommands.java b/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsCommands.java index 2815e4a2a3..77102e10e4 100644 --- a/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsCommands.java +++ b/java/client/src/main/java/glide/api/commands/ScriptingAndFunctionsCommands.java @@ -142,4 +142,19 @@ public interface ScriptingAndFunctionsCommands { * } */ CompletableFuture fcall(String function); + + /** + * Kills a function that is currently executing.
+ * FUNCTION KILL terminates read-only functions only. + * + * @since Redis 7.0 and above. + * @see redis.io for details. + * @return OK if function is terminated. Otherwise, throws an error. + * @example + * 
{@code
+     * String response = client.functionKill().get();
+     * assert response.equals("OK");
+     * }
+ */ + CompletableFuture functionKill(); } diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java index 4f4cebf27e..0a557fc904 100644 --- a/java/client/src/main/java/glide/api/models/BaseTransaction.java +++ b/java/client/src/main/java/glide/api/models/BaseTransaction.java @@ -53,6 +53,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.GeoAdd; @@ -3788,6 +3789,20 @@ public T functionList(@NonNull String libNamePattern, boolean withCode) { return getThis(); } + /** + * Kills a function that is currently executing.
+ * FUNCTION KILL terminates read-only functions only. + * + * @since Redis 7.0 and above. + * @see redis.io for details. + * @return Command Response - OK if function is terminated. Otherwise, throws an + * error. + */ + public T functionKill() { + protobufTransaction.addCommands(buildCommand(FunctionKill)); + return getThis(); + } + /** * Invokes a previously loaded function. * diff --git a/java/client/src/test/java/glide/api/RedisClientTest.java b/java/client/src/test/java/glide/api/RedisClientTest.java index 79c95d644e..01bd48d988 100644 --- a/java/client/src/test/java/glide/api/RedisClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClientTest.java @@ -85,6 +85,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.GeoAdd; @@ -5226,6 +5227,27 @@ public void fcall_returns_success() { assertEquals(value, payload); } + @SneakyThrows + @Test + public void functionKill_returns_success() { + // setup + String[] args = new String[0]; + CompletableFuture testResponse = new CompletableFuture<>(); + testResponse.complete(OK); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(FunctionKill), eq(args), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.functionKill(); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(OK, payload); + } + @SneakyThrows @Test public void bitcount_returns_success() { diff --git a/java/client/src/test/java/glide/api/RedisClusterClientTest.java b/java/client/src/test/java/glide/api/RedisClusterClientTest.java index 15ff0a59fd..46cf670800 100644 --- a/java/client/src/test/java/glide/api/RedisClusterClientTest.java +++ b/java/client/src/test/java/glide/api/RedisClusterClientTest.java @@ -30,6 +30,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.Info; @@ -1543,4 +1544,46 @@ public void fcall_without_keys_and_with_route_returns_success() { assertEquals(testResponse, response); assertEquals(value, payload); } + + @SneakyThrows + @Test + public void functionKill_returns_success() { + // setup + String[] args = new String[0]; + CompletableFuture testResponse = new CompletableFuture<>(); + testResponse.complete(OK); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(FunctionKill), eq(args), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.functionKill(); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(OK, payload); + } + + @SneakyThrows + @Test + public void functionKill_with_route_returns_success() { + // setup + String[] args = new String[0]; + CompletableFuture testResponse = new CompletableFuture<>(); + testResponse.complete(OK); + + // match on protobuf request + when(commandManager.submitNewCommand(eq(FunctionKill), eq(args), eq(RANDOM), any())) + .thenReturn(testResponse); + + // exercise + CompletableFuture response = service.functionKill(RANDOM); + String payload = response.get(); + + // verify + assertEquals(testResponse, response); + assertEquals(OK, payload); + } } diff --git a/java/client/src/test/java/glide/api/models/TransactionTests.java b/java/client/src/test/java/glide/api/models/TransactionTests.java index a5564a332a..3c16c817ed 100644 --- a/java/client/src/test/java/glide/api/models/TransactionTests.java +++ b/java/client/src/test/java/glide/api/models/TransactionTests.java @@ -66,6 +66,7 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; +import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.GeoAdd; @@ -874,6 +875,9 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)), transaction.fcall("func", new String[] {"arg1", "arg2"}); results.add(Pair.of(FCall, buildArgs("func", "0", "arg1", "arg2"))); + transaction.functionKill(); + results.add(Pair.of(FunctionKill, buildArgs())); + transaction.geodist("key", "Place", "Place2"); results.add(Pair.of(GeoDist, buildArgs("key", "Place", "Place2"))); transaction.geodist("key", "Place", "Place2", GeoUnit.KILOMETERS); diff --git a/java/integTest/src/test/java/glide/TestUtilities.java b/java/integTest/src/test/java/glide/TestUtilities.java index 9032f7f048..73b52bb3dc 100644 --- a/java/integTest/src/test/java/glide/TestUtilities.java +++ b/java/integTest/src/test/java/glide/TestUtilities.java @@ -181,4 +181,36 @@ public static String generateLuaLibCode( } return code.toString(); } + + /** + * Create a lua lib with a RO function which runs an endless loop up to timeout sec.
+ * Execution takes at least 5 sec regardless of the timeout configured.
+ * If readOnly is false, function sets a dummy value to the first key + * given. + */ + public static String createLuaLibWithLongRunningFunction( + String libName, String funcName, int timeout, boolean readOnly) { + String code = + "#!lua name=$libName\n" + + "local function $libName_$funcName(keys, args)\n" + + " local started = tonumber(redis.pcall('time')[1])\n" + // fun fact - redis does no writes if 'no-writes' flag is set + + " redis.pcall('set', keys[1], 42)\n" + + " while (true) do\n" + + " local now = tonumber(redis.pcall('time')[1])\n" + + " if now > started + $timeout then\n" + + " return 'Timed out $timeout sec'\n" + + " end\n" + + " end\n" + + " return 'OK'\n" + + "end\n" + + "redis.register_function{\n" + + "function_name='$funcName',\n" + + "callback=$libName_$funcName,\n" + + (readOnly ? "flags={ 'no-writes' }\n" : "") + + "}"; + return code.replace("$timeout", Integer.toString(timeout)) + .replace("$funcName", funcName) + .replace("$libName", libName); + } } diff --git a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java index 958460b1fb..c2ae091ba5 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java @@ -7,7 +7,9 @@ import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleSingleNodeRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -20,10 +22,12 @@ import glide.api.models.configuration.RequestRoutingConfiguration.SingleNodeRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotIdRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotType; +import glide.api.models.exceptions.RequestException; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -176,4 +180,20 @@ public void zrank_zrevrank_withscores() { assertArrayEquals(new Object[] {0L, 1.0}, (Object[]) result[1]); assertArrayEquals(new Object[] {2L, 1.0}, (Object[]) result[2]); } + + @Test + @SneakyThrows + public void functionKill() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + ClusterTransaction transaction = new ClusterTransaction().functionKill(); + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> clusterClient.exec(transaction).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // can't test a successfull call to FUNCTION KILL in a transaction, + // because Redis rejects transaction when blocked by a function/script + } } diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 5df004426b..b1f104709d 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1,10 +1,11 @@ /** Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.cluster; -import static glide.TestConfiguration.CLUSTER_PORTS; import static glide.TestConfiguration.REDIS_VERSION; import static glide.TestUtilities.assertDeepEquals; import static glide.TestUtilities.checkFunctionListResponse; +import static glide.TestUtilities.commonClusterClientConfig; +import static glide.TestUtilities.createLuaLibWithLongRunningFunction; import static glide.TestUtilities.generateLuaLibCode; import static glide.TestUtilities.getFirstEntryFromMultiValue; import static glide.TestUtilities.getValueFromInfo; @@ -47,8 +48,6 @@ import glide.api.models.commands.RangeOptions.RangeByIndex; import glide.api.models.commands.WeightAggregateOptions.KeyArray; import glide.api.models.commands.bitmap.BitwiseOperation; -import glide.api.models.configuration.NodeAddress; -import glide.api.models.configuration.RedisClusterClientConfiguration; import glide.api.models.configuration.RequestRoutingConfiguration.Route; import glide.api.models.configuration.RequestRoutingConfiguration.SingleNodeRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotKeyRoute; @@ -131,11 +130,7 @@ public class CommandTests { @SneakyThrows public static void init() { clusterClient = - RedisClusterClient.CreateClient( - RedisClusterClientConfiguration.builder() - .address(NodeAddress.builder().port(CLUSTER_PORTS[0]).build()) - .requestTimeout(5000) - .build()) + RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) .get(); } @@ -1097,4 +1092,334 @@ public void fcall_readonly_function() { assertEquals(OK, clusterClient.functionDelete(libName).get()); } + + @Test + @SneakyThrows + public void functionStats_and_functionKill_without_route() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill_without_route"; + String funcName = "deadlock_without_route"; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 15, true); + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, clusterClient.functionLoad(code, true).get()); + + try (var testClient = + RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) + .get()) { + // call the function without await + // TODO use FCALL + // Using a random primary node route, otherwise FCALL can go to a replica. + // FKILL and FSTATS go to primary nodes if no route given, test fails in such case. + Route route = new SlotKeyRoute(UUID.randomUUID().toString(), PRIMARY); + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}, route); + + int timeout = 5200; // ms + while (timeout > 0) { + var stats = clusterClient.customCommand(new String[] {"FUNCTION", "STATS"}).get(); + boolean found = false; + for (var response : stats.getMultiValue().values()) { + if (((Map) response).get("running_script") != null) { + found = true; + break; + } + } + if (found) { + break; + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + assertEquals(OK, clusterClient.functionKill().get()); + + exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + exception = assertThrows(ExecutionException.class, promise::get); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().contains("Script killed by user")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + clusterClient.functionKill().get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should be killed before."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, + clusterClient + .customCommand(new String[] {"FUNCTION", "DELETE", libName}) + .get() + .getSingleValue()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } + + @ParameterizedTest(name = "single node route = {0}") + @ValueSource(booleans = {true, false}) + @SneakyThrows + public void functionStats_and_functionKill_with_route(boolean singleNodeRoute) { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill_with_route_" + singleNodeRoute; + String funcName = "deadlock_with_route_" + singleNodeRoute; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 15, true); + Route route = + singleNodeRoute ? new SlotKeyRoute(UUID.randomUUID().toString(), PRIMARY) : ALL_PRIMARIES; + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, clusterClient.functionLoad(code, true, route).get()); + + try (var testClient = + RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) + .get()) { + // call the function without await + // TODO use FCALL + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}, route); + + int timeout = 5200; // ms + while (timeout > 0) { + var stats = clusterClient.customCommand(new String[] {"FUNCTION", "STATS"}, route).get(); + if (singleNodeRoute) { + var response = stats.getSingleValue(); + if (((Map) response).get("running_script") != null) { + break; + } + } else { + boolean found = false; + for (var response : stats.getMultiValue().values()) { + if (((Map) response).get("running_script") != null) { + found = true; + break; + } + } + if (found) { + break; + } + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + // redis kills a function with 5 sec delay + assertEquals(OK, clusterClient.functionKill(route).get()); + Thread.sleep(404); + + exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + exception = assertThrows(ExecutionException.class, promise::get); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().contains("Script killed by user")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + clusterClient.functionKill(route).get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should be killed before."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, + clusterClient + .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) + .get() + .getSingleValue()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } + + @Test + @SneakyThrows + public void functionStats_and_functionKill_with_key_based_route() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill_with_key_based_route"; + String funcName = "deadlock_with_key_based_route"; + String key = libName; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 15, true); + Route route = new SlotKeyRoute(key, PRIMARY); + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, clusterClient.functionLoad(code, true, route).get()); + + try (var testClient = + RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) + .get()) { + // call the function without await + // TODO use FCALL + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", key}); + + int timeout = 5200; // ms + while (timeout > 0) { + var stats = clusterClient.customCommand(new String[] {"FUNCTION", "STATS"}, route).get(); + var response = stats.getSingleValue(); + if (((Map) response).get("running_script") != null) { + break; + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + // redis kills a function with 5 sec delay + assertEquals(OK, clusterClient.functionKill(route).get()); + + exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + exception = assertThrows(ExecutionException.class, promise::get); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().contains("Script killed by user")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + clusterClient.functionKill(route).get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should be killed before."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, + clusterClient + .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) + .get() + .getSingleValue()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } + + @Test + @SneakyThrows + public void functionStats_and_functionKill_write_function() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill_write_function"; + String funcName = "deadlock_write_function_with_key_based_route"; + String key = libName; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 6, false); + Route route = new SlotKeyRoute(key, PRIMARY); + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, clusterClient.functionLoadReplace(code, route).get()); + + try (var testClient = + RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) + .get()) { + // call the function without await + // TODO use FCALL + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", key}); + + int timeout = 5200; // ms + while (timeout > 0) { + var stats = clusterClient.customCommand(new String[] {"FUNCTION", "STATS"}, route).get(); + var response = stats.getSingleValue(); + if (((Map) response).get("running_script") != null) { + break; + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + // redis kills a function with 5 sec delay + exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("unkillable")); + + assertEquals("Timed out 6 sec", promise.get().getSingleValue()); + + exception = + assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + clusterClient.functionKill(route).get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should finish prior to the test end."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, + clusterClient + .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) + .get() + .getSingleValue()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } } diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 1ec5835a89..8617c945ba 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -2,8 +2,9 @@ package glide.standalone; import static glide.TestConfiguration.REDIS_VERSION; -import static glide.TestConfiguration.STANDALONE_PORTS; import static glide.TestUtilities.checkFunctionListResponse; +import static glide.TestUtilities.commonClientConfig; +import static glide.TestUtilities.createLuaLibWithLongRunningFunction; import static glide.TestUtilities.generateLuaLibCode; import static glide.TestUtilities.getValueFromInfo; import static glide.TestUtilities.parseInfoResponseToMap; @@ -29,8 +30,6 @@ import glide.api.RedisClient; import glide.api.models.commands.InfoOptions; -import glide.api.models.configuration.NodeAddress; -import glide.api.models.configuration.RedisClientConfiguration; import glide.api.models.exceptions.RequestException; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -57,11 +56,7 @@ public class CommandTests { @SneakyThrows public static void init() { regularClient = - RedisClient.CreateClient( - RedisClientConfiguration.builder() - .address(NodeAddress.builder().port(STANDALONE_PORTS[0]).build()) - .build()) - .get(); + RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get(); } @AfterAll @@ -518,4 +513,143 @@ public void copy() { regularClient.select(0).get(); } } + + @Test + @SneakyThrows + public void functionStats_and_functionKill() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill"; + String funcName = "deadlock"; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 15, true); + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> regularClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, regularClient.functionLoad(code, true).get()); + + try (var testClient = + RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get()) { + // call the function without await + // TODO use FCALL + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}); + + int timeout = 5200; // ms + while (timeout > 0) { + var response = regularClient.customCommand(new String[] {"FUNCTION", "STATS"}).get(); + if (((Map) response).get("running_script") != null) { + break; + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + // redis kills a function with 5 sec delay + assertEquals(OK, regularClient.functionKill().get()); + + exception = + assertThrows(ExecutionException.class, () -> regularClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + exception = assertThrows(ExecutionException.class, promise::get); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().contains("Script killed by user")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + regularClient.functionKill().get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should be killed before."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, regularClient.customCommand(new String[] {"FUNCTION", "DELETE", libName}).get()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } + + @Test + @SneakyThrows + public void functionStats_and_functionKill_write_function() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + String libName = "functionStats_and_functionKill_write_function"; + String funcName = "deadlock_write_function"; + String code = createLuaLibWithLongRunningFunction(libName, funcName, 6, false); + String error = ""; + + try { + // nothing to kill + var exception = + assertThrows(ExecutionException.class, () -> regularClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // load the lib + assertEquals(libName, regularClient.functionLoad(code, true).get()); + + try (var testClient = + RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get()) { + // call the function without await + // TODO use FCALL + var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", libName}); + + int timeout = 5200; // ms + while (timeout > 0) { + var response = regularClient.customCommand(new String[] {"FUNCTION", "STATS"}).get(); + if (((Map) response).get("running_script") != null) { + break; + } + Thread.sleep(100); + timeout -= 100; + } + if (timeout == 0) { + error += "Can't find a running function."; + } + + // can't kill a write function + exception = + assertThrows(ExecutionException.class, () -> regularClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("unkillable")); + + assertEquals("Timed out 6 sec", promise.get()); + + exception = + assertThrows(ExecutionException.class, () -> regularClient.functionKill().get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + } + } finally { + // If function wasn't killed, and it didn't time out - it blocks the server and cause rest + // test to fail. + try { + regularClient.functionKill().get(); + // should throw `notbusy` error, because the function should be killed before + error += "Function should finish prior to the test end."; + } catch (Exception ignored) { + } + } + + // TODO replace with FUNCTION DELETE + assertEquals( + OK, regularClient.customCommand(new String[] {"FUNCTION", "DELETE", libName}).get()); + + assertTrue(error.isEmpty(), "Something went wrong during the test"); + } } diff --git a/java/integTest/src/test/java/glide/standalone/TransactionTests.java b/java/integTest/src/test/java/glide/standalone/TransactionTests.java index a0adf6c8e0..928603511e 100644 --- a/java/integTest/src/test/java/glide/standalone/TransactionTests.java +++ b/java/integTest/src/test/java/glide/standalone/TransactionTests.java @@ -3,25 +3,27 @@ import static glide.TestConfiguration.REDIS_VERSION; import static glide.TestUtilities.assertDeepEquals; +import static glide.TestUtilities.commonClientConfig; import static glide.api.BaseClient.OK; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -import glide.TestConfiguration; import glide.TransactionTestUtilities.TransactionBuilder; import glide.api.RedisClient; import glide.api.models.Transaction; import glide.api.models.commands.InfoOptions; -import glide.api.models.configuration.NodeAddress; -import glide.api.models.configuration.RedisClientConfiguration; +import glide.api.models.exceptions.RequestException; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -38,13 +40,7 @@ public class TransactionTests { @BeforeAll @SneakyThrows public static void init() { - client = - RedisClient.CreateClient( - RedisClientConfiguration.builder() - .address( - NodeAddress.builder().port(TestConfiguration.STANDALONE_PORTS[0]).build()) - .build()) - .get(); + client = RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get(); } @AfterAll @@ -264,4 +260,18 @@ public void copy() { Object[] result = client.exec(transaction).get(); assertArrayEquals(expectedResult, result); } + + @Test + public void functionKill() { + assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); + + Transaction transaction = new Transaction().functionKill(); + // nothing to kill + var exception = assertThrows(ExecutionException.class, () -> client.exec(transaction).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); + + // can't test a successfull call to FUNCTION KILL in a transaction, + // because Redis rejects transaction when blocked by a function/script + } } From 9c049fb98605ff54dac2ddedffaaa9ed749a68c6 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 12 Jun 2024 17:37:44 -0700 Subject: [PATCH 2/4] Remove transaction implementation and tests. Signed-off-by: Yury-Fridlyand --- .../glide/api/models/BaseTransaction.java | 15 -------------- .../glide/api/models/TransactionTests.java | 4 ---- .../cluster/ClusterTransactionTests.java | 20 ------------------- .../glide/standalone/TransactionTests.java | 18 ----------------- 4 files changed, 57 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/BaseTransaction.java b/java/client/src/main/java/glide/api/models/BaseTransaction.java index 0a557fc904..4f4cebf27e 100644 --- a/java/client/src/main/java/glide/api/models/BaseTransaction.java +++ b/java/client/src/main/java/glide/api/models/BaseTransaction.java @@ -53,7 +53,6 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; -import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.GeoAdd; @@ -3789,20 +3788,6 @@ public T functionList(@NonNull String libNamePattern, boolean withCode) { return getThis(); } - /** - * Kills a function that is currently executing.
- * FUNCTION KILL terminates read-only functions only. - * - * @since Redis 7.0 and above. - * @see redis.io for details. - * @return Command Response - OK if function is terminated. Otherwise, throws an - * error. - */ - public T functionKill() { - protobufTransaction.addCommands(buildCommand(FunctionKill)); - return getThis(); - } - /** * Invokes a previously loaded function. * diff --git a/java/client/src/test/java/glide/api/models/TransactionTests.java b/java/client/src/test/java/glide/api/models/TransactionTests.java index 3c16c817ed..a5564a332a 100644 --- a/java/client/src/test/java/glide/api/models/TransactionTests.java +++ b/java/client/src/test/java/glide/api/models/TransactionTests.java @@ -66,7 +66,6 @@ import static redis_request.RedisRequestOuterClass.RequestType.FlushAll; import static redis_request.RedisRequestOuterClass.RequestType.FunctionDelete; import static redis_request.RedisRequestOuterClass.RequestType.FunctionFlush; -import static redis_request.RedisRequestOuterClass.RequestType.FunctionKill; import static redis_request.RedisRequestOuterClass.RequestType.FunctionList; import static redis_request.RedisRequestOuterClass.RequestType.FunctionLoad; import static redis_request.RedisRequestOuterClass.RequestType.GeoAdd; @@ -875,9 +874,6 @@ InfScoreBound.NEGATIVE_INFINITY, new ScoreBoundary(3, false), new Limit(1, 2)), transaction.fcall("func", new String[] {"arg1", "arg2"}); results.add(Pair.of(FCall, buildArgs("func", "0", "arg1", "arg2"))); - transaction.functionKill(); - results.add(Pair.of(FunctionKill, buildArgs())); - transaction.geodist("key", "Place", "Place2"); results.add(Pair.of(GeoDist, buildArgs("key", "Place", "Place2"))); transaction.geodist("key", "Place", "Place2", GeoUnit.KILOMETERS); diff --git a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java index c2ae091ba5..958460b1fb 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java @@ -7,9 +7,7 @@ import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleSingleNodeRoute.RANDOM; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -22,12 +20,10 @@ import glide.api.models.configuration.RequestRoutingConfiguration.SingleNodeRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotIdRoute; import glide.api.models.configuration.RequestRoutingConfiguration.SlotType; -import glide.api.models.exceptions.RequestException; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Map; import java.util.UUID; -import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -180,20 +176,4 @@ public void zrank_zrevrank_withscores() { assertArrayEquals(new Object[] {0L, 1.0}, (Object[]) result[1]); assertArrayEquals(new Object[] {2L, 1.0}, (Object[]) result[2]); } - - @Test - @SneakyThrows - public void functionKill() { - assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); - - ClusterTransaction transaction = new ClusterTransaction().functionKill(); - // nothing to kill - var exception = - assertThrows(ExecutionException.class, () -> clusterClient.exec(transaction).get()); - assertInstanceOf(RequestException.class, exception.getCause()); - assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); - - // can't test a successfull call to FUNCTION KILL in a transaction, - // because Redis rejects transaction when blocked by a function/script - } } diff --git a/java/integTest/src/test/java/glide/standalone/TransactionTests.java b/java/integTest/src/test/java/glide/standalone/TransactionTests.java index 928603511e..5ca4019d6c 100644 --- a/java/integTest/src/test/java/glide/standalone/TransactionTests.java +++ b/java/integTest/src/test/java/glide/standalone/TransactionTests.java @@ -8,9 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -18,12 +16,10 @@ import glide.api.RedisClient; import glide.api.models.Transaction; import glide.api.models.commands.InfoOptions; -import glide.api.models.exceptions.RequestException; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Map; import java.util.UUID; -import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -260,18 +256,4 @@ public void copy() { Object[] result = client.exec(transaction).get(); assertArrayEquals(expectedResult, result); } - - @Test - public void functionKill() { - assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7"); - - Transaction transaction = new Transaction().functionKill(); - // nothing to kill - var exception = assertThrows(ExecutionException.class, () -> client.exec(transaction).get()); - assertInstanceOf(RequestException.class, exception.getCause()); - assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); - - // can't test a successfull call to FUNCTION KILL in a transaction, - // because Redis rejects transaction when blocked by a function/script - } } From 630b9130c0efeffea47d198f06c8c8f055367b04 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 12 Jun 2024 17:42:47 -0700 Subject: [PATCH 3/4] Fix the rebase. Signed-off-by: Yury-Fridlyand --- .../test/java/glide/cluster/CommandTests.java | 16 ++++++---------- .../test/java/glide/standalone/CommandTests.java | 6 ++---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index b1f104709d..66eacecf44 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1117,11 +1117,10 @@ public void functionStats_and_functionKill_without_route() { RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) .get()) { // call the function without await - // TODO use FCALL // Using a random primary node route, otherwise FCALL can go to a replica. // FKILL and FSTATS go to primary nodes if no route given, test fails in such case. Route route = new SlotKeyRoute(UUID.randomUUID().toString(), PRIMARY); - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}, route); + var promise = testClient.fcall(funcName, route); int timeout = 5200; // ms while (timeout > 0) { @@ -1203,8 +1202,7 @@ public void functionStats_and_functionKill_with_route(boolean singleNodeRoute) { RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) .get()) { // call the function without await - // TODO use FCALL - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}, route); + var promise = testClient.fcall(funcName, route); int timeout = 5200; // ms while (timeout > 0) { @@ -1294,8 +1292,7 @@ public void functionStats_and_functionKill_with_key_based_route() { RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) .get()) { // call the function without await - // TODO use FCALL - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", key}); + var promise = testClient.fcall(funcName, new String[] {key}, new String[0]); int timeout = 5200; // ms while (timeout > 0) { @@ -1365,14 +1362,13 @@ public void functionStats_and_functionKill_write_function() { assertTrue(exception.getMessage().toLowerCase().contains("notbusy")); // load the lib - assertEquals(libName, clusterClient.functionLoadReplace(code, route).get()); + assertEquals(libName, clusterClient.functionLoad(code, true, route).get()); try (var testClient = RedisClusterClient.CreateClient(commonClusterClientConfig().requestTimeout(7000).build()) .get()) { // call the function without await - // TODO use FCALL - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", key}); + var promise = testClient.fcall(funcName, new String[] {key}, new String[0]); int timeout = 5200; // ms while (timeout > 0) { @@ -1394,7 +1390,7 @@ public void functionStats_and_functionKill_write_function() { assertInstanceOf(RequestException.class, exception.getCause()); assertTrue(exception.getMessage().toLowerCase().contains("unkillable")); - assertEquals("Timed out 6 sec", promise.get().getSingleValue()); + assertEquals("Timed out 6 sec", promise.get()); exception = assertThrows(ExecutionException.class, () -> clusterClient.functionKill(route).get()); diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 8617c945ba..4c57be350d 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -537,8 +537,7 @@ public void functionStats_and_functionKill() { try (var testClient = RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get()) { // call the function without await - // TODO use FCALL - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "0"}); + var promise = testClient.fcall(funcName); int timeout = 5200; // ms while (timeout > 0) { @@ -606,8 +605,7 @@ public void functionStats_and_functionKill_write_function() { try (var testClient = RedisClient.CreateClient(commonClientConfig().requestTimeout(7000).build()).get()) { // call the function without await - // TODO use FCALL - var promise = testClient.customCommand(new String[] {"FCALL", funcName, "1", libName}); + var promise = testClient.fcall(funcName, new String[] {libName}, new String[0]); int timeout = 5200; // ms while (timeout > 0) { From d3d6ef73eeeaa5b40e32cc8037eb788349970473 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 12 Jun 2024 17:59:33 -0700 Subject: [PATCH 4/4] Remove TODOs. Signed-off-by: Yury-Fridlyand --- .../test/java/glide/cluster/CommandTests.java | 32 +++---------------- .../java/glide/standalone/CommandTests.java | 8 ++--- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 66eacecf44..1cc8502a19 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1164,13 +1164,7 @@ public void functionStats_and_functionKill_without_route() { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, - clusterClient - .customCommand(new String[] {"FUNCTION", "DELETE", libName}) - .get() - .getSingleValue()); + assertEquals(OK, clusterClient.functionDelete(libName).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); } @@ -1255,13 +1249,7 @@ public void functionStats_and_functionKill_with_route(boolean singleNodeRoute) { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, - clusterClient - .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) - .get() - .getSingleValue()); + assertEquals(OK, clusterClient.functionDelete(libName, route).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); } @@ -1331,13 +1319,7 @@ public void functionStats_and_functionKill_with_key_based_route() { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, - clusterClient - .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) - .get() - .getSingleValue()); + assertEquals(OK, clusterClient.functionDelete(libName, route).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); } @@ -1408,13 +1390,7 @@ public void functionStats_and_functionKill_write_function() { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, - clusterClient - .customCommand(new String[] {"FUNCTION", "DELETE", libName}, route) - .get() - .getSingleValue()); + assertEquals(OK, clusterClient.functionDelete(libName, route).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); } diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java index 4c57be350d..432be4e89d 100644 --- a/java/integTest/src/test/java/glide/standalone/CommandTests.java +++ b/java/integTest/src/test/java/glide/standalone/CommandTests.java @@ -575,9 +575,7 @@ public void functionStats_and_functionKill() { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, regularClient.customCommand(new String[] {"FUNCTION", "DELETE", libName}).get()); + assertEquals(OK, regularClient.functionDelete(libName).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); } @@ -644,9 +642,7 @@ public void functionStats_and_functionKill_write_function() { } } - // TODO replace with FUNCTION DELETE - assertEquals( - OK, regularClient.customCommand(new String[] {"FUNCTION", "DELETE", libName}).get()); + assertEquals(OK, regularClient.functionDelete(libName).get()); assertTrue(error.isEmpty(), "Something went wrong during the test"); }