Java: Add transaction commands #895

acarbonetto · 2024-02-02T05:47:15Z

Issue #, if available:

Description of changes:

Adds transactions for both standalone and cluster clients using the exec command: by passing a route (for cluster clients) and a pipeline of commands in a Transaction object. The object is then passed to Redis through the submitNewCommand(Transaction) call.

This PR is supposed for merge after #894

Examples:
// Transactions
Transaction standaloneTransaction = new Transaction()
    .ping()
    .set("key", "value")
    .get("key")
    .info();
Object[] transactionResult = client.exec(standaloneTransaction).get();

ClusterTransaction clusterTransaction = new ClusterTransaction()
    .ping()
    .set("key", "value")
    .get("key")
    .info();
ClusterValue[] transactionResult = clusterClient.exec(clusterTransaction, ALL_NODES).get();

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

java/client/src/main/java/glide/api/models/BaseTransaction.java

java/client/src/main/java/glide/api/models/configuration/RequestRoutingConfiguration.java

java/client/src/main/java/glide/managers/CommandManager.java

java/client/src/test/java/glide/api/RedisClientTest.java

java/client/src/test/java/glide/managers/CommandManagerTest.java

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

java/client/src/main/java/glide/api/models/BaseTransaction.java

Yury-Fridlyand

Please add

IT
protection for custom command vs empty args input

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

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

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

shachlanAmazon · 2024-02-05T14:08:17Z

java/client/src/main/java/glide/api/models/BaseTransaction.java

+@Getter
+public abstract class BaseTransaction<T extends BaseTransaction<T>> {
+    /** Command class to send a single request to Redis. */
+    protected final Transaction.Builder transactionBuilder = Transaction.newBuilder();


You're using here protobuf's Transaction, and it reduces legibility. Please use ProtobufTransaction, or a similar name.

java/client/src/main/java/glide/api/models/BaseTransaction.java

java/client/src/test/java/glide/managers/CommandManagerTest.java

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

java/client/src/main/java/glide/api/models/BaseTransaction.java

acarbonetto · 2024-02-07T06:13:50Z

java/client/src/main/java/glide/api/models/commands/SetOptions.java

+ * @see <a href="https://redis.io/commands/set/">redis.io</a>
+ */
+@Builder
+public final class SetOptions {


This will be updated from changes in #894.

java/client/src/test/java/glide/managers/CommandManagerTest.java

shachlanAmazon

approved, assuming the set/info specific code is removed.

shachlanAmazon · 2024-02-08T09:08:28Z

java/client/src/main/java/glide/api/models/commands/SetOptions.java

+ * @see <a href="https://redis.io/commands/set/">redis.io</a>
+ */
+@Builder
+public final class SetOptions {


can this be remove from this PR?
You can add it after the set PR is merged

I was actually intending to merge this after the set() command PR. There's no rush on this one.

shachlanAmazon · 2024-02-08T09:09:23Z

java/client/src/main/java/glide/api/models/commands/InfoOptions.java

+ * @see <a href="https://redis.io/commands/info/">redis.io</a>
+ */
+@Builder
+public final class InfoOptions {


can this be removed from this PR? you can add it after the info command PR is merged.

I was actually intending to merge this after the info() command PR. There's no rush on this one.

java/integTest/src/test/java/glide/standalone/TransactionTests.java

java/client/src/main/java/glide/managers/CommandManager.java

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

Yury-Fridlyand · 2024-02-12T18:09:16Z

java/integTest/src/test/java/glide/cluster/ClusterTransactionTests.java

+                clusterClient.exec(transaction, RANDOM).get(10, TimeUnit.SECONDS);
+        Object[] results =
+                Arrays.stream(clusterValues)
+                        .map(v -> v.hasSingleData() ? v.getSingleValue() : v.getMultiValue())


Should be always multivalue for cluster isn't it?

depends on the route

sorry, I missed this - transactions only support routing to single nodes, so there's no need to use ClusterValue.

I will submit a follow-up PR to fix this then. Thanks for the comment.

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto requested a review from a team as a code owner February 2, 2024 05:47

acarbonetto self-assigned this Feb 2, 2024

acarbonetto added the java issues and fixes related to the java client label Feb 2, 2024

Yury-Fridlyand reviewed Feb 2, 2024

View reviewed changes

java/client/src/main/java/glide/api/BaseClient.java Outdated Show resolved Hide resolved

Yury-Fridlyand reviewed Feb 2, 2024

View reviewed changes

java/client/src/main/java/glide/api/models/BaseTransaction.java Show resolved Hide resolved

Yury-Fridlyand approved these changes Feb 3, 2024

View reviewed changes

ikolomi self-requested a review February 5, 2024 10:28

ikolomi approved these changes Feb 5, 2024

View reviewed changes

java/client/src/main/java/glide/api/commands/BaseCommands.java Outdated Show resolved Hide resolved

java/client/src/main/java/glide/api/commands/ClusterBaseCommands.java Outdated Show resolved Hide resolved

shachlanAmazon reviewed Feb 5, 2024

View reviewed changes

acarbonetto commented Feb 7, 2024

View reviewed changes

shachlanAmazon approved these changes Feb 8, 2024

View reviewed changes

acarbonetto force-pushed the java/integ_acarbo_api_transactions_only branch from b5fc521 to c98feb2 Compare February 11, 2024 09:27

shachlanAmazon approved these changes Feb 11, 2024

View reviewed changes

Yury-Fridlyand approved these changes Feb 12, 2024

View reviewed changes

acarbonetto and others added 14 commits February 12, 2024 11:51

Java: Add transaction commands

e7f5be5

Signed-off-by: Andrew Carbonetto <[email protected]>

Clean up for review comments

a85e5aa

Signed-off-by: Andrew Carbonetto <[email protected]>

Fix CommandManagerTest.java

7ec1331

Signed-off-by: Andrew Carbonetto <[email protected]>

All transactions require an argument (empty is fine)

190186e

Signed-off-by: Andrew Carbonetto <[email protected]>

Add IT tests for transactions

1f21f83

Signed-off-by: Andrew Carbonetto <[email protected]>

Renaming field

600b5e9

Signed-off-by: Andrew Carbonetto <[email protected]>

Add IT tests for Transactions

6a2323e

Signed-off-by: Andrew Carbonetto <[email protected]>

Update exec() command with route

81107ee

Signed-off-by: Andrew Carbonetto <[email protected]>

Update exec() command with route

e0decd2

Signed-off-by: Andrew Carbonetto <[email protected]>

Remove failing tests

3c6e038

Signed-off-by: Andrew Carbonetto <[email protected]>

Spotless

1c8a701

Signed-off-by: Andrew Carbonetto <[email protected]>

Spotless

43db9ff

Signed-off-by: Andrew Carbonetto <[email protected]>

Spotless

9063e7d

Signed-off-by: Andrew Carbonetto <[email protected]>

Update cluster comments

e25c55f

Signed-off-by: Andrew Carbonetto <[email protected]>

acarbonetto force-pushed the java/integ_acarbo_api_transactions_only branch from 8dc38f6 to e25c55f Compare February 12, 2024 20:08

acarbonetto merged commit 8823a1a into valkey-io:main Feb 12, 2024
11 checks passed

Yury-Fridlyand deleted the java/integ_acarbo_api_transactions_only branch February 12, 2024 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java: Add transaction commands #895

Java: Add transaction commands #895

acarbonetto commented Feb 2, 2024 •

edited by Yury-Fridlyand

Loading

Yury-Fridlyand left a comment

shachlanAmazon Feb 5, 2024

acarbonetto Feb 7, 2024

shachlanAmazon left a comment

shachlanAmazon Feb 8, 2024

acarbonetto Feb 8, 2024

shachlanAmazon Feb 8, 2024

acarbonetto Feb 8, 2024

Yury-Fridlyand Feb 12, 2024

acarbonetto Feb 12, 2024

shachlanAmazon Feb 12, 2024

acarbonetto Feb 12, 2024

Java: Add transaction commands #895

Java: Add transaction commands #895

Conversation

acarbonetto commented Feb 2, 2024 • edited by Yury-Fridlyand Loading

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shachlanAmazon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acarbonetto commented Feb 2, 2024 •

edited by Yury-Fridlyand

Loading