Java: AZ Awareness #2678

tjzhang-BQ · 2024-11-13T01:19:32Z

Issue link

This Pull Request is linked to issue (URL): #643

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one issue.
Commit message has a detailed description of what changed and why.
Tests are added or updated.
CHANGELOG.md and documentation files are updated.
Destination branch is correct - main or release
Commits will be squashed upon merging.

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

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

java/integTest/src/test/java/glide/AzAwarenessTests.java

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

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

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

java/integTest/src/test/java/glide/ConnectionTests.java

Signed-off-by: TJ Zhang <[email protected]>

java/integTest/src/test/java/glide/ConnectionTests.java

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

java/integTest/src/test/java/glide/ConnectionTests.java

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

Yury-Fridlyand · 2024-11-15T20:33:58Z

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

I prefer to have structure like

interface IReadFrom {} enum ReadFrom implements IReadFrom { PRIMARY, REPLICA }; class AZ implements IReadFrom { String AZ };

It is more more correct, but will be a breaking change.

Furthermore we need 2 different ReadFrom - for standalone and cluster. AZ option is not applicable for standalone.

AZ is applicable for standalone

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

Yury-Fridlyand · 2024-11-15T20:41:30Z

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

@@ -122,6 +123,14 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderBaseConfiguration
            connectionRequestBuilder.setInflightRequestsLimit(configuration.getInflightRequestsLimit());
        }

+        if (configuration.getReadFrom() == ReadFrom.AZ_AFFINITY) {
+            if (configuration.getClientAZ() != null) {


Throw if building config for standalone client. Unfortunately, we can't restrict this in compile-time without introducing a breaking change.

java/integTest/src/test/java/glide/ConnectionTests.java

Signed-off-by: TJ Zhang <[email protected]>

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

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

Signed-off-by: TJ Zhang <[email protected]>

acarbonetto · 2024-11-20T21:22:55Z

java/integTest/build.gradle

@@ -84,6 +85,25 @@ tasks.register('startCluster') {
    }
 }

+tasks.register('startClusterForAz') {


Really, we can use this cluster for any test, not just az awareness.
We should consider renaming this to something like startClusterWithMultipleReplicaNodes or just startLargeCluster

not required for this PR, but let's consider splitting out our regular IT test framework from tests that require 'more setup'.

Alternatively, we can move cluster setup to the IT test framework and let individual test files set up clusters

acarbonetto · 2024-11-20T21:27:02Z

java/integTest/src/test/java/glide/TestUtilities.java

@@ -111,6 +112,17 @@ public static Map<String, String> parseInfoResponseToMap(String serverInfo) {
        return builder.useTLS(TLS);
    }

+    public static GlideClusterClientConfiguration.GlideClusterClientConfigurationBuilder<?, ?>


lots of common code between GlideClientConfigurationBuilder, GlideClusterClientConfigurationBuilder and GlideClusterClientConfigurationBuilder

acarbonetto · 2024-11-20T21:28:27Z

java/integTest/src/test/java/glide/ConnectionTests.java

+                                new String[] {"INFO", "REPLICATION"},
+                                new RequestRoutingConfiguration.SlotKeyRoute("key", PRIMARY))
+                        .get();
+        long nReplicas =


* Java: AZ awareness Signed-off-by: TJ Zhang <[email protected]>

tjzhang-BQ added the java issues and fixes related to the java client label Nov 13, 2024

tjzhang-BQ force-pushed the java/tjz-azawareness branch from 5a8ecb8 to 34f1c0a Compare November 13, 2024 01:45

acarbonetto reviewed Nov 13, 2024

View reviewed changes

java/client/src/main/java/glide/managers/ConnectionManager.java Outdated Show resolved Hide resolved

acarbonetto reviewed Nov 14, 2024

View reviewed changes

java/client/src/test/java/glide/managers/ConnectionManagerTest.java Show resolved Hide resolved

acarbonetto reviewed Nov 14, 2024

View reviewed changes

java/integTest/src/test/java/glide/AzAwarenessTests.java Outdated Show resolved Hide resolved

jonathanl-bq reviewed Nov 14, 2024

View reviewed changes

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

tjzhang-BQ force-pushed the java/tjz-azawareness branch from 65baf97 to cb101f3 Compare November 15, 2024 03:10

tjzhang-BQ marked this pull request as ready for review November 15, 2024 03:31

tjzhang-BQ requested a review from a team as a code owner November 15, 2024 03:31

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 15, 2024 20:10 — with GitHub Actions Error

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 15, 2024 20:22 — with GitHub Actions Error

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 15, 2024 23:10 — with GitHub Actions Error

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 15, 2024 23:27 — with GitHub Actions Error

acarbonetto mentioned this pull request Nov 16, 2024

AZ Awareness #643

Closed

tjzhang-BQ force-pushed the java/tjz-azawareness branch from ca8bf19 to 621a57c Compare November 16, 2024 00:56

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 16, 2024 01:02 — with GitHub Actions Error

acarbonetto reviewed Nov 18, 2024

View reviewed changes

acarbonetto approved these changes Nov 18, 2024

View reviewed changes

tjzhang-BQ commented Nov 18, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Show resolved Hide resolved

tjzhang-BQ requested a deployment to AWS_ACTIONS November 18, 2024 09:09 — with GitHub Actions Waiting

TJ Zhang added 4 commits November 18, 2024 12:18

Java: AZ awareness

1e64176

Signed-off-by: TJ Zhang <[email protected]>

updating config builder

c4a65ac

Signed-off-by: TJ Zhang <[email protected]>

add tests

9809213

Signed-off-by: TJ Zhang <[email protected]>

address comment

00fe70e

Signed-off-by: TJ Zhang <[email protected]>

tjzhang-BQ force-pushed the java/tjz-azawareness branch from d7561ec to 00fe70e Compare November 18, 2024 20:19

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 18, 2024 20:19 — with GitHub Actions Error

jonathanl-bq reviewed Nov 19, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Outdated Show resolved Hide resolved

jonathanl-bq approved these changes Nov 19, 2024

View reviewed changes

adarovadya reviewed Nov 20, 2024

View reviewed changes

java/client/src/main/java/glide/api/models/configuration/ReadFrom.java Outdated Show resolved Hide resolved

adarovadya reviewed Nov 20, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Show resolved Hide resolved

Yury-Fridlyand reviewed Nov 20, 2024

View reviewed changes

adarovadya reviewed Nov 20, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Outdated Show resolved Hide resolved

adarovadya reviewed Nov 20, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Outdated Show resolved Hide resolved

adarovadya reviewed Nov 20, 2024

View reviewed changes

java/integTest/src/test/java/glide/ConnectionTests.java Outdated Show resolved Hide resolved

add separate test cluster for az & address comments

3470637

Signed-off-by: TJ Zhang <[email protected]>

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 20, 2024 20:11 — with GitHub Actions Error

updating error message

950d525

Signed-off-by: TJ Zhang <[email protected]>

tjzhang-BQ had a problem deploying to AWS_ACTIONS November 20, 2024 20:13 — with GitHub Actions Error

acarbonetto reviewed Nov 20, 2024

View reviewed changes

java/client/src/main/java/glide/managers/ConnectionManager.java Outdated Show resolved Hide resolved

java/client/src/test/java/glide/managers/ConnectionManagerTest.java Outdated Show resolved Hide resolved

updating tests

6b60b0b

Signed-off-by: TJ Zhang <[email protected]>

tjzhang-BQ requested a deployment to AWS_ACTIONS November 20, 2024 20:50 — with GitHub Actions Waiting

acarbonetto approved these changes Nov 20, 2024

View reviewed changes

tjzhang-BQ merged commit 408ca25 into release-1.2 Nov 20, 2024
15 of 16 checks passed

tjzhang-BQ deleted the java/tjz-azawareness branch November 20, 2024 23:00

prateek-kumar-improving pushed a commit that referenced this pull request Nov 26, 2024

Java: AZ Awareness (#2678)

07e55c6

* Java: AZ awareness Signed-off-by: TJ Zhang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java: AZ Awareness #2678

Java: AZ Awareness #2678

tjzhang-BQ commented Nov 13, 2024 •

edited by acarbonetto

Loading

Yury-Fridlyand Nov 15, 2024

Yury-Fridlyand Nov 15, 2024

adarovadya Nov 20, 2024

Yury-Fridlyand Nov 15, 2024

acarbonetto Nov 20, 2024

acarbonetto Nov 20, 2024

acarbonetto Nov 20, 2024

acarbonetto Nov 20, 2024

acarbonetto Nov 20, 2024

Java: AZ Awareness #2678

Java: AZ Awareness #2678

Conversation

tjzhang-BQ commented Nov 13, 2024 • edited by acarbonetto Loading

Issue link

Checklist

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

tjzhang-BQ commented Nov 13, 2024 •

edited by acarbonetto

Loading