From f1167c17c576ed71562622e9750a7ac7df36cf1a Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 4 Jul 2024 16:51:27 -0700 Subject: [PATCH] Java: store pointers in scripts (#1795) Store script args and keys as pointers. Signed-off-by: Yury-Fridlyand --- .../src/main/java/glide/api/BaseClient.java | 8 ++--- .../main/java/glide/api/models/Script.java | 10 +++--- .../java/glide/managers/CommandManager.java | 22 ++++++++++-- .../test/java/glide/SharedCommandTests.java | 36 +++++++++++++++++++ 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 46b13dea32..b09be22bb1 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -3,8 +3,6 @@ import static glide.api.models.GlideString.gs; import static glide.api.models.commands.SortBaseOptions.STORE_COMMAND_STRING; -import static glide.api.models.commands.bitmap.BitFieldOptions.BitFieldReadOnlySubCommands; -import static glide.api.models.commands.bitmap.BitFieldOptions.BitFieldSubCommands; import static glide.api.models.commands.bitmap.BitFieldOptions.createBitFieldArgs; import static glide.api.models.commands.bitmap.BitFieldOptions.createBitFieldGlideStringArgs; import static glide.api.models.commands.stream.StreamClaimOptions.JUST_ID_REDIS_API; @@ -1765,7 +1763,7 @@ public CompletableFuture pexpiretime(@NonNull GlideString key) { @Override public CompletableFuture invokeScript(@NonNull Script script) { - if (script.getBinarySafeOutput()) { + if (script.getBinaryOutput()) { return commandManager.submitScript( script, List.of(), List.of(), this::handleBinaryObjectOrNullResponse); } else { @@ -1777,7 +1775,7 @@ public CompletableFuture invokeScript(@NonNull Script script) { @Override public CompletableFuture invokeScript( @NonNull Script script, @NonNull ScriptOptions options) { - if (script.getBinarySafeOutput()) { + if (script.getBinaryOutput()) { return commandManager.submitScript( script, options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()), @@ -1795,7 +1793,7 @@ public CompletableFuture invokeScript( @Override public CompletableFuture invokeScript( @NonNull Script script, @NonNull ScriptOptionsGlideString options) { - if (script.getBinarySafeOutput()) { + if (script.getBinaryOutput()) { return commandManager.submitScript( script, options.getKeys(), options.getArgs(), this::handleBinaryObjectOrNullResponse); } else { diff --git a/java/client/src/main/java/glide/api/models/Script.java b/java/client/src/main/java/glide/api/models/Script.java index bec0ec6960..6a074e2573 100644 --- a/java/client/src/main/java/glide/api/models/Script.java +++ b/java/client/src/main/java/glide/api/models/Script.java @@ -20,18 +20,18 @@ public class Script implements AutoCloseable { private boolean isDropped = false; - /** Indicatoin if script invocation output can return binary data. */ - @Getter private final Boolean binarySafeOutput; + /** Indication if script invocation output can return binary data. */ + @Getter private final Boolean binaryOutput; /** * Wraps around creating a Script object from code. * * @param code To execute with a ScriptInvoke call. - * @param binarySafeOutput Indicates if the output can return binary data. + * @param binaryOutput Indicates if the output can return binary data. */ - public Script(T code, Boolean binarySafeOutput) { + public Script(T code, Boolean binaryOutput) { this.hash = storeScript(GlideString.of(code).getBytes()); - this.binarySafeOutput = binarySafeOutput; + this.binaryOutput = binaryOutput; } /** Drop the linked script from glide-rs code. */ diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 77595c6af9..34f52dd6b4 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -33,6 +33,7 @@ import redis_request.RedisRequestOuterClass.RequestType; import redis_request.RedisRequestOuterClass.Routes; import redis_request.RedisRequestOuterClass.ScriptInvocation; +import redis_request.RedisRequestOuterClass.ScriptInvocationPointers; import redis_request.RedisRequestOuterClass.SimpleRoutes; import redis_request.RedisRequestOuterClass.SlotTypes; import response.ResponseOuterClass.Response; @@ -161,7 +162,7 @@ public CompletableFuture submitScript( List args, RedisExceptionCheckedFunction responseHandler) { - RedisRequest.Builder command = prepareRedisRequest(script, keys, args); + RedisRequest.Builder command = prepareScript(script, keys, args); return submitCommandToChannel(command, responseHandler); } @@ -285,8 +286,25 @@ protected RedisRequest.Builder prepareRedisRequest(Transaction transaction) { * @return An uncompleted request. {@link CallbackDispatcher} is responsible to complete it by * adding a callback id. */ - protected RedisRequest.Builder prepareRedisRequest( + protected RedisRequest.Builder prepareScript( Script script, List keys, List args) { + + if (keys.stream().mapToLong(key -> key.getBytes().length).sum() + + args.stream().mapToLong(key -> key.getBytes().length).sum() + > RedisValueResolver.MAX_REQUEST_ARGS_LENGTH_IN_BYTES) { + return RedisRequest.newBuilder() + .setScriptInvocationPointers( + ScriptInvocationPointers.newBuilder() + .setHash(script.getHash()) + .setArgsPointer( + RedisValueResolver.createLeakedBytesVec( + args.stream().map(GlideString::getBytes).toArray(byte[][]::new))) + .setKeysPointer( + RedisValueResolver.createLeakedBytesVec( + keys.stream().map(GlideString::getBytes).toArray(byte[][]::new))) + .build()); + } + return RedisRequest.newBuilder() .setScriptInvocation( ScriptInvocation.newBuilder() diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java index 0fd23ce940..a294683710 100644 --- a/java/integTest/src/test/java/glide/SharedCommandTests.java +++ b/java/integTest/src/test/java/glide/SharedCommandTests.java @@ -3065,6 +3065,42 @@ public void invokeScript_test(BaseClient client) { } } + @SneakyThrows + @ParameterizedTest(autoCloseArguments = false) + @MethodSource("getClients") + public void script_large_keys_and_or_args(BaseClient client) { + String str1 = "0".repeat(1 << 12); // 4k + String str2 = "0".repeat(1 << 12); // 4k + + try (Script script = new Script("return KEYS[1]", false)) { + // 1 very big key + Object response = + client.invokeScript(script, ScriptOptions.builder().key(str1 + str2).build()).get(); + assertEquals(str1 + str2, response); + } + + try (Script script = new Script("return KEYS[1]", false)) { + // 2 big keys + Object response = + client.invokeScript(script, ScriptOptions.builder().key(str1).key(str2).build()).get(); + assertEquals(str1, response); + } + + try (Script script = new Script("return ARGV[1]", false)) { + // 1 very big arg + Object response = + client.invokeScript(script, ScriptOptions.builder().arg(str1 + str2).build()).get(); + assertEquals(str1 + str2, response); + } + + try (Script script = new Script("return ARGV[1]", false)) { + // 1 big arg + 1 big key + Object response = + client.invokeScript(script, ScriptOptions.builder().arg(str1).key(str2).build()).get(); + assertEquals(str2, response); + } + } + @SneakyThrows @ParameterizedTest(autoCloseArguments = false) @MethodSource("getClients")