From 93387e12e1cdbc8bf1c67ac96d214241f1c945d6 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 24 Sep 2024 11:12:56 -0700 Subject: [PATCH 1/8] Signed-off-by: Yury-Fridlyand --- .github/workflows/java.yml | 2 +- .../src/test/java/glide/cluster/CommandTests.java | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/java.yml b/.github/workflows/java.yml index 5106aec4ce..2b82821a39 100644 --- a/.github/workflows/java.yml +++ b/.github/workflows/java.yml @@ -98,7 +98,7 @@ jobs: - name: Build java client working-directory: java - run: ./gradlew --continue build -x javadoc + run: ./gradlew :integTest:test --tests '*.fcall_readonly_function' - name: Ensure no skipped files by linter working-directory: java diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index cadd37a707..81cb7e594e 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -100,6 +100,7 @@ import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; @@ -1646,12 +1647,13 @@ public void fcall_binary_with_keys(String prefix) { } @SneakyThrows - @Test + // @Test + @RepeatedTest(1000) public void fcall_readonly_function() { assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in version 7"); - assumeTrue( - !SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), - "Temporary disabeling this test on valkey 8"); + // assumeTrue( + // !SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), + // "Temporary disabeling this test on valkey 8"); String libName = "fcall_readonly_function"; // intentionally using a REPLICA route @@ -1663,6 +1665,7 @@ public void fcall_readonly_function() { String code = generateLuaLibCode(libName, Map.of(funcName, "return 42"), false); assertEquals(libName, clusterClient.functionLoad(code, false).get()); + Thread.sleep(1000); // fcall on a replica node should fail, because a function isn't guaranteed to be RO var executionException = From 30e3661918fa77514eee63d98f5efa4f0c89f100 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 24 Sep 2024 11:53:46 -0700 Subject: [PATCH 2/8] Signed-off-by: Yury-Fridlyand --- .github/workflows/java.yml | 2 +- .../test/java/glide/cluster/CommandTests.java | 49 ++++++++----------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/.github/workflows/java.yml b/.github/workflows/java.yml index 2b82821a39..5106aec4ce 100644 --- a/.github/workflows/java.yml +++ b/.github/workflows/java.yml @@ -98,7 +98,7 @@ jobs: - name: Build java client working-directory: java - run: ./gradlew :integTest:test --tests '*.fcall_readonly_function' + run: ./gradlew --continue build -x javadoc - name: Ensure no skipped files by linter working-directory: java diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 81cb7e594e..075ed7176b 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -100,7 +100,6 @@ import lombok.SneakyThrows; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; @@ -1605,55 +1604,47 @@ public void fcall_binary_with_keys(String prefix) { assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in version 7"); String key = "{" + prefix + "}-fcall_with_keys-"; + GlideString binaryString = + gs(new byte[] {(byte) 0xFE, (byte) 0xEE, (byte) 0xEF, (byte) 252, (byte) 0}); SingleNodeRoute route = new SlotKeyRoute(key, PRIMARY); String libName = "mylib_with_keys"; GlideString funcName = gs("myfunc_with_keys"); // function $funcName returns array with first two arguments String code = - generateLuaLibCode(libName, Map.of(funcName.toString(), "return {keys[1], keys[2]}"), true); + generateLuaLibCode(libName, Map.of(funcName.toString(), "return {args[1]}"), true); // loading function to the node where key is stored assertEquals(libName, clusterClient.functionLoad(code, false, route).get()); - // due to common prefix, all keys are mapped to the same hash slot var functionResult = clusterClient - .fcall(funcName, new GlideString[] {gs(key + 1), gs(key + 2)}, new GlideString[0]) + .fcall(funcName, new GlideString[] {gs(key)}, new GlideString[] {binaryString}) .get(); - assertArrayEquals(new Object[] {gs(key + 1), gs(key + 2)}, (Object[]) functionResult); + assertArrayEquals(new Object[] {binaryString}, (Object[]) functionResult); functionResult = clusterClient - .fcallReadOnly( - funcName, new GlideString[] {gs(key + 1), gs(key + 2)}, new GlideString[0]) + .fcallReadOnly(funcName, new GlideString[] {gs(key)}, new GlideString[] {binaryString}) .get(); - assertArrayEquals(new Object[] {gs(key + 1), gs(key + 2)}, (Object[]) functionResult); - - // TODO: change to binary transaction version once available: - // var transaction = - // new ClusterTransaction() - // .fcall(funcName, new String[] {key + 1, key + 2}, new String[0]) - // .fcallReadOnly(funcName, new String[] {key + 1, key + 2}, new String[0]); - - // // check response from a routed transaction request - // assertDeepEquals( - // new Object[][] {{key + 1, key + 2}, {key + 1, key + 2}}, - // clusterClient.exec(transaction, route).get()); - // // if no route given, GLIDE should detect it automatically - // assertDeepEquals( - // new Object[][] {{key + 1, key + 2}, {key + 1, key + 2}}, - // clusterClient.exec(transaction).get()); + assertArrayEquals(new Object[] {binaryString}, (Object[]) functionResult); + + var transaction = + new ClusterTransaction() + .withBinaryOutput() + .fcall(funcName, new GlideString[] {gs(key)}, new GlideString[] {binaryString}) + .fcallReadOnly(funcName, new GlideString[] {gs(key)}, new GlideString[] {binaryString}); + + // check response from a routed transaction request + assertDeepEquals(new Object[][] {{binaryString}}, clusterClient.exec(transaction, route).get()); + // if no route given, GLIDE should detect it automatically + assertDeepEquals(new Object[][] {{binaryString}}, clusterClient.exec(transaction).get()); assertEquals(OK, clusterClient.functionDelete(libName, route).get()); } @SneakyThrows - // @Test - @RepeatedTest(1000) + @Test public void fcall_readonly_function() { assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in version 7"); - // assumeTrue( - // !SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), - // "Temporary disabeling this test on valkey 8"); String libName = "fcall_readonly_function"; // intentionally using a REPLICA route @@ -1665,7 +1656,7 @@ public void fcall_readonly_function() { String code = generateLuaLibCode(libName, Map.of(funcName, "return 42"), false); assertEquals(libName, clusterClient.functionLoad(code, false).get()); - Thread.sleep(1000); + Thread.sleep(1000); // let replica sync with the primary node // fcall on a replica node should fail, because a function isn't guaranteed to be RO var executionException = From 7c02646d1673d3455ea74df10813e381c4595ff7 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 24 Sep 2024 12:15:44 -0700 Subject: [PATCH 3/8] Signed-off-by: Yury-Fridlyand --- .../src/test/java/glide/cluster/CommandTests.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 075ed7176b..f15c5221db 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1607,8 +1607,8 @@ public void fcall_binary_with_keys(String prefix) { GlideString binaryString = gs(new byte[] {(byte) 0xFE, (byte) 0xEE, (byte) 0xEF, (byte) 252, (byte) 0}); SingleNodeRoute route = new SlotKeyRoute(key, PRIMARY); - String libName = "mylib_with_keys"; - GlideString funcName = gs("myfunc_with_keys"); + String libName = "mylib_with_keys-" + prefix; + GlideString funcName = gs("myfunc_with_keys-" + prefix); // function $funcName returns array with first two arguments String code = generateLuaLibCode(libName, Map.of(funcName.toString(), "return {args[1]}"), true); @@ -1634,9 +1634,12 @@ public void fcall_binary_with_keys(String prefix) { .fcallReadOnly(funcName, new GlideString[] {gs(key)}, new GlideString[] {binaryString}); // check response from a routed transaction request - assertDeepEquals(new Object[][] {{binaryString}}, clusterClient.exec(transaction, route).get()); + assertDeepEquals( + new Object[][] {{binaryString}, {binaryString}}, + clusterClient.exec(transaction, route).get()); // if no route given, GLIDE should detect it automatically - assertDeepEquals(new Object[][] {{binaryString}}, clusterClient.exec(transaction).get()); + assertDeepEquals( + new Object[][] {{binaryString}, {binaryString}}, clusterClient.exec(transaction).get()); assertEquals(OK, clusterClient.functionDelete(libName, route).get()); } From 5daea17a258bc0a138aeba8a970555eceb976f9f Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 24 Sep 2024 12:33:44 -0700 Subject: [PATCH 4/8] Signed-off-by: Yury-Fridlyand --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index f15c5221db..38b71f4ca6 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1607,8 +1607,8 @@ public void fcall_binary_with_keys(String prefix) { GlideString binaryString = gs(new byte[] {(byte) 0xFE, (byte) 0xEE, (byte) 0xEF, (byte) 252, (byte) 0}); SingleNodeRoute route = new SlotKeyRoute(key, PRIMARY); - String libName = "mylib_with_keys-" + prefix; - GlideString funcName = gs("myfunc_with_keys-" + prefix); + String libName = "mylib_with_keys_" + prefix; + GlideString funcName = gs("myfunc_with_keys_" + prefix); // function $funcName returns array with first two arguments String code = generateLuaLibCode(libName, Map.of(funcName.toString(), "return {args[1]}"), true); From e153e5f23fc6adc52b26666bfd17314e49174e79 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 26 Sep 2024 11:25:05 -0700 Subject: [PATCH 5/8] Update java/integTest/src/test/java/glide/cluster/CommandTests.java Co-authored-by: Andrew Carbonetto Signed-off-by: Yury-Fridlyand --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 38b71f4ca6..c86ade8764 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1609,7 +1609,7 @@ public void fcall_binary_with_keys(String prefix) { SingleNodeRoute route = new SlotKeyRoute(key, PRIMARY); String libName = "mylib_with_keys_" + prefix; GlideString funcName = gs("myfunc_with_keys_" + prefix); - // function $funcName returns array with first two arguments + // function $funcName returns array with first argument String code = generateLuaLibCode(libName, Map.of(funcName.toString(), "return {args[1]}"), true); From a83de345e095a6c7a5a4673ec1225c2c08584319 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 2 Oct 2024 18:09:57 -0700 Subject: [PATCH 6/8] --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index c86ade8764..dbe1affef4 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1659,7 +1659,8 @@ public void fcall_readonly_function() { String code = generateLuaLibCode(libName, Map.of(funcName, "return 42"), false); assertEquals(libName, clusterClient.functionLoad(code, false).get()); - Thread.sleep(1000); // let replica sync with the primary node + // let replica sync with the primary node + assertEquals(1L, clusterClient.wait(1L, 1000L).get()); // fcall on a replica node should fail, because a function isn't guaranteed to be RO var executionException = From c5a1978fb9ffbd84488301992601bf0fb2aec4d8 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 3 Oct 2024 15:36:54 -0700 Subject: [PATCH 7/8] --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index dbe1affef4..28462beb14 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1660,7 +1660,7 @@ public void fcall_readonly_function() { assertEquals(libName, clusterClient.functionLoad(code, false).get()); // let replica sync with the primary node - assertEquals(1L, clusterClient.wait(1L, 1000L).get()); + assertEquals(1L, clusterClient.wait(1L, 2000L).get()); // fcall on a replica node should fail, because a function isn't guaranteed to be RO var executionException = From 16cc3708c46e11fbf4c12fd2f38642b2cd99cb0d Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 3 Oct 2024 16:34:46 -0700 Subject: [PATCH 8/8] --- java/integTest/src/test/java/glide/cluster/CommandTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java index 28462beb14..8cbdabdb99 100644 --- a/java/integTest/src/test/java/glide/cluster/CommandTests.java +++ b/java/integTest/src/test/java/glide/cluster/CommandTests.java @@ -1660,7 +1660,7 @@ public void fcall_readonly_function() { assertEquals(libName, clusterClient.functionLoad(code, false).get()); // let replica sync with the primary node - assertEquals(1L, clusterClient.wait(1L, 2000L).get()); + assertEquals(1L, clusterClient.wait(1L, 3000L).get()); // fcall on a replica node should fail, because a function isn't guaranteed to be RO var executionException =