From 02d1d5a8caa00ff9bcee847d024f32cc7064912a Mon Sep 17 00:00:00 2001 From: annie-mac Date: Tue, 8 Nov 2022 14:54:54 -0800 Subject: [PATCH 1/4] set requestContext flag properly --- .../DocumentServiceRequestContext.java | 2 +- .../AddressEnumeratorTests.java | 145 ++++++++++-------- 2 files changed, 79 insertions(+), 68 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DocumentServiceRequestContext.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DocumentServiceRequestContext.java index e87892e5eaf9..e614bc950a4a 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DocumentServiceRequestContext.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DocumentServiceRequestContext.java @@ -41,7 +41,7 @@ public class DocumentServiceRequestContext implements Cloneable { public volatile CosmosDiagnostics cosmosDiagnostics; public volatile String resourcePhysicalAddress; public volatile String throughputControlCycleId; - public volatile boolean replicaAddressValidationEnabled; + public volatile boolean replicaAddressValidationEnabled = Configs.isReplicaAddressValidationEnabled(); private final Set failedEndpoints = ConcurrentHashMap.newKeySet(); public DocumentServiceRequestContext() {} diff --git a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/AddressEnumeratorTests.java b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/AddressEnumeratorTests.java index 1205ac845f35..8f8d23c95edf 100644 --- a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/AddressEnumeratorTests.java +++ b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/AddressEnumeratorTests.java @@ -29,106 +29,117 @@ public class AddressEnumeratorTests { @Test(groups = "unit") public void replicaAddressValidationEnabledComparatorTests() throws NoSuchMethodException, NoSuchFieldException, IllegalAccessException, InvocationTargetException { - Method sortAddressesMethod = AddressEnumerator.class.getDeclaredMethod("sortAddresses", List.class, RxDocumentServiceRequest.class); - sortAddressesMethod.setAccessible(true); + try { + System.setProperty("COSMOS.REPLICA_ADDRESS_VALIDATION_ENABLED", "true"); + Method sortAddressesMethod = AddressEnumerator.class.getDeclaredMethod("sortAddresses", List.class, RxDocumentServiceRequest.class); + sortAddressesMethod.setAccessible(true); - // set a different health status to each endpoint to test the sorting logic - Uri testUri1 = new Uri("https://127.0.0.1:1"); - assertThat(testUri1.getHealthStatus()).isEqualTo(Unknown); + // set a different health status to each endpoint to test the sorting logic + Uri testUri1 = new Uri("https://127.0.0.1:1"); + assertThat(testUri1.getHealthStatus()).isEqualTo(Unknown); - Uri testUri2 = new Uri("https://127.0.0.1:2"); - testUri2.setConnected(); - assertThat(testUri2.getHealthStatus()).isEqualTo(Connected); + Uri testUri2 = new Uri("https://127.0.0.1:2"); + testUri2.setConnected(); + assertThat(testUri2.getHealthStatus()).isEqualTo(Connected); - Uri testUri3 = new Uri("https://127.0.0.1:3"); - testUri3.setUnhealthy(); - testUri3.setRefreshed(); - assertThat(testUri3.getHealthStatus()).isEqualTo(UnhealthyPending); + Uri testUri3 = new Uri("https://127.0.0.1:3"); + testUri3.setUnhealthy(); + testUri3.setRefreshed(); + assertThat(testUri3.getHealthStatus()).isEqualTo(UnhealthyPending); - Uri testUri4 = new Uri("https://127.0.0.1:4"); - testUri4.setUnhealthy(); - assertThat(testUri4.getHealthStatus()).isEqualTo(Unhealthy); + Uri testUri4 = new Uri("https://127.0.0.1:4"); + testUri4.setUnhealthy(); + assertThat(testUri4.getHealthStatus()).isEqualTo(Unhealthy); - RxDocumentServiceRequest requestMock = Mockito.mock(RxDocumentServiceRequest.class); - requestMock.requestContext = new DocumentServiceRequestContext(); - requestMock.requestContext.replicaAddressValidationEnabled = true; + RxDocumentServiceRequest requestMock = Mockito.mock(RxDocumentServiceRequest.class); + requestMock.requestContext = new DocumentServiceRequestContext(); - // when replicaAddressValidation is enabled, we prefer Connected/Unknown > UnhealthyPending > Unhealthy - List testScenarios = Arrays.asList( + // when replicaAddressValidation is enabled, we prefer Connected/Unknown > UnhealthyPending > Unhealthy + List testScenarios = Arrays.asList( new SortAddressesTestScenario(Arrays.asList(testUri1, testUri2), Arrays.asList(testUri1, testUri2)), // unknown, connected -> unknown, connected new SortAddressesTestScenario(Arrays.asList(testUri2, testUri1), Arrays.asList(testUri2, testUri1)), // connected, unknown -> connected, unknown new SortAddressesTestScenario(Arrays.asList(testUri3, testUri2), Arrays.asList(testUri2, testUri3)), // unhealthyPending, connected -> connected, unhealthyPending new SortAddressesTestScenario(Arrays.asList(testUri4, testUri1), Arrays.asList(testUri1, testUri4)), // unhealthy, unknown -> unknown, unhealthy new SortAddressesTestScenario(Arrays.asList(testUri4, testUri3), Arrays.asList(testUri3, testUri4))); // unhealthy, unhealthyPending -> unhealthyPending, unhealthy - for (SortAddressesTestScenario testScenario : testScenarios) { - System.out.println("Test scenario, comparing " + testScenario.getAddresses()); - for (Uri uri : testScenario.getAddresses()) { - this.setTimestamp(uri, Instant.now()); - } - List sortedAddresses = + for (SortAddressesTestScenario testScenario : testScenarios) { + System.out.println("Test scenario, comparing " + testScenario.getAddresses()); + for (Uri uri : testScenario.getAddresses()) { + this.setTimestamp(uri, Instant.now()); + } + List sortedAddresses = (List) sortAddressesMethod.invoke(null, testScenario.getAddresses(), requestMock); - assertThat(sortedAddresses).containsExactlyElementsOf(testScenario.expectedAddresses); - } + assertThat(sortedAddresses).containsExactlyElementsOf(testScenario.expectedAddresses); + } - System.out.println("Test scenario: comparing when unhealthyPending roll into healthy status after 1 min"); - setTimestamp(testUri3, Instant.now().minusMillis(Duration.ofMinutes(2).toMillis())); - List sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri3, testUri2), requestMock); - assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri3, testUri2)); + System.out.println("Test scenario: comparing when unhealthyPending roll into healthy status after 1 min"); + setTimestamp(testUri3, Instant.now().minusMillis(Duration.ofMinutes(2).toMillis())); + List sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri3, testUri2), requestMock); + assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri3, testUri2)); + + System.out.println("Test scenario: comparing when there is failedEndpoints marked in request context"); + requestMock.requestContext.addToFailedEndpoints(new GoneException("Test"), testUri2); + sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri4, testUri2), requestMock); + assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri4, testUri2)); + } finally { + System.clearProperty("COSMOS.REPLICA_ADDRESS_VALIDATION_ENABLED"); + } - System.out.println("Test scenario: comparing when there is failedEndpoints marked in request context"); - requestMock.requestContext.addToFailedEndpoints(new GoneException("Test"), testUri2); - sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri4, testUri2), requestMock); - assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri4, testUri2)); } @Test(groups = "unit") public void replicaAddressValidationDisabledComparatorTests() throws NoSuchMethodException, NoSuchFieldException, IllegalAccessException, InvocationTargetException { - Method sortAddressesMethod = AddressEnumerator.class.getDeclaredMethod("sortAddresses", List.class, RxDocumentServiceRequest.class); - sortAddressesMethod.setAccessible(true); + try { + System.setProperty("COSMOS.REPLICA_ADDRESS_VALIDATION_ENABLED", "false"); + Method sortAddressesMethod = AddressEnumerator.class.getDeclaredMethod("sortAddresses", List.class, RxDocumentServiceRequest.class); + sortAddressesMethod.setAccessible(true); - // set a different health status to each endpoint to test the sorting logic - Uri testUri1 = new Uri("https://127.0.0.1:1"); - assertThat(testUri1.getHealthStatus()).isEqualTo(Unknown); + // set a different health status to each endpoint to test the sorting logic + Uri testUri1 = new Uri("https://127.0.0.1:1"); + assertThat(testUri1.getHealthStatus()).isEqualTo(Unknown); - Uri testUri2 = new Uri("https://127.0.0.1:2"); - testUri2.setConnected(); - assertThat(testUri2.getHealthStatus()).isEqualTo(Connected); + Uri testUri2 = new Uri("https://127.0.0.1:2"); + testUri2.setConnected(); + assertThat(testUri2.getHealthStatus()).isEqualTo(Connected); - Uri testUri3 = new Uri("https://127.0.0.1:3"); - testUri3.setUnhealthy(); - testUri3.setRefreshed(); - assertThat(testUri3.getHealthStatus()).isEqualTo(UnhealthyPending); + Uri testUri3 = new Uri("https://127.0.0.1:3"); + testUri3.setUnhealthy(); + testUri3.setRefreshed(); + assertThat(testUri3.getHealthStatus()).isEqualTo(UnhealthyPending); - Uri testUri4 = new Uri("https://127.0.0.1:4"); - testUri4.setUnhealthy(); - assertThat(testUri4.getHealthStatus()).isEqualTo(Unhealthy); + Uri testUri4 = new Uri("https://127.0.0.1:4"); + testUri4.setUnhealthy(); + assertThat(testUri4.getHealthStatus()).isEqualTo(Unhealthy); - RxDocumentServiceRequest requestMock = Mockito.mock(RxDocumentServiceRequest.class); - requestMock.requestContext = new DocumentServiceRequestContext(); + RxDocumentServiceRequest requestMock = Mockito.mock(RxDocumentServiceRequest.class); + requestMock.requestContext = new DocumentServiceRequestContext(); - // when replicaAddressValidation is enabled, we prefer Connected/Unknown/UnhealthyPending > Unhealthy - List testScenarios = Arrays.asList( + // when replicaAddressValidation is enabled, we prefer Connected/Unknown/UnhealthyPending > Unhealthy + List testScenarios = Arrays.asList( new SortAddressesTestScenario(Arrays.asList(testUri1, testUri2), Arrays.asList(testUri1, testUri2)), // unknown, connected -> unknown, connected new SortAddressesTestScenario(Arrays.asList(testUri2, testUri1), Arrays.asList(testUri2, testUri1)), // connected, unknown -> connected, unknown new SortAddressesTestScenario(Arrays.asList(testUri3, testUri2), Arrays.asList(testUri3, testUri2)), // unhealthyPending, connected -> unhealthyPending, connected new SortAddressesTestScenario(Arrays.asList(testUri4, testUri1), Arrays.asList(testUri1, testUri4)), // unhealthy, unknown -> unknown, unhealthy new SortAddressesTestScenario(Arrays.asList(testUri4, testUri3), Arrays.asList(testUri3, testUri4))); // unhealthy, unhealthyPending -> unhealthyPending, unhealthy - for (SortAddressesTestScenario testScenario : testScenarios) { - System.out.println("Test scenario, comparing " + testScenario.getAddresses()); - for (Uri uri : testScenario.getAddresses()) { - this.setTimestamp(uri, Instant.now()); - } - List sortedAddresses = + for (SortAddressesTestScenario testScenario : testScenarios) { + System.out.println("Test scenario, comparing " + testScenario.getAddresses()); + for (Uri uri : testScenario.getAddresses()) { + this.setTimestamp(uri, Instant.now()); + } + List sortedAddresses = (List) sortAddressesMethod.invoke(null, testScenario.getAddresses(), requestMock); - assertThat(sortedAddresses).containsExactlyElementsOf(testScenario.expectedAddresses); + assertThat(sortedAddresses).containsExactlyElementsOf(testScenario.expectedAddresses); + } + + System.out.println("Test scenario: comparing when there is failedEndpoints marked in request context"); + requestMock.requestContext.addToFailedEndpoints(new GoneException("Test"), testUri2); + List sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri4, testUri2), requestMock); + assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri4, testUri2)); + } finally { + System.clearProperty("COSMOS.REPLICA_ADDRESS_VALIDATION_ENABLED"); } - System.out.println("Test scenario: comparing when there is failedEndpoints marked in request context"); - requestMock.requestContext.addToFailedEndpoints(new GoneException("Test"), testUri2); - List sortedAddresses = (List) sortAddressesMethod.invoke(null, Arrays.asList(testUri4, testUri2), requestMock); - assertThat(sortedAddresses).containsExactlyElementsOf(Arrays.asList(testUri4, testUri2)); } private void setTimestamp(Uri testUri, Instant time) throws NoSuchFieldException, IllegalAccessException { @@ -172,4 +183,4 @@ public List getExpectedAddresses() { return expectedAddresses; } } -} \ No newline at end of file +} From da6511ae761f6d48df4ddc7762f6c27195c3ff13 Mon Sep 17 00:00:00 2001 From: annie-mac Date: Tue, 8 Nov 2022 15:04:42 -0800 Subject: [PATCH 2/4] add changelog --- sdk/cosmos/azure-cosmos/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/cosmos/azure-cosmos/CHANGELOG.md b/sdk/cosmos/azure-cosmos/CHANGELOG.md index fe2100884d45..f53cb9501910 100644 --- a/sdk/cosmos/azure-cosmos/CHANGELOG.md +++ b/sdk/cosmos/azure-cosmos/CHANGELOG.md @@ -9,6 +9,7 @@ #### Bugs Fixed * Fixed a rare race condition for `query plan` cache exceeding the allowed size limit - See [PR 31859](https://github.com/Azure/azure-sdk-for-java/pull/31859) * Added improvement in `RntbdClientChannelHealthChecker` for detecting continuous transit timeout. - See [PR 31544](https://github.com/Azure/azure-sdk-for-java/pull/31544) +* Fix an issue in replica validation where addresses sorting does not happen properly when replica validation is enabled. - See [PR 32022](https://github.com/Azure/azure-sdk-for-java/pull/32022) #### Other Changes * Shaded `MurmurHash3` of apache `commons-codec` to enable removing of the `guava` dependency - CVE-2020-8908 - See [PR 31761](https://github.com/Azure/azure-sdk-for-java/pull/31761) From 4147a110b18a7d7c873df46bd54af8f18a20da45 Mon Sep 17 00:00:00 2001 From: annie-mac Date: Tue, 8 Nov 2022 15:18:21 -0800 Subject: [PATCH 3/4] update changelog --- sdk/cosmos/azure-cosmos/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/cosmos/azure-cosmos/CHANGELOG.md b/sdk/cosmos/azure-cosmos/CHANGELOG.md index f53cb9501910..cf8853c4540a 100644 --- a/sdk/cosmos/azure-cosmos/CHANGELOG.md +++ b/sdk/cosmos/azure-cosmos/CHANGELOG.md @@ -9,7 +9,7 @@ #### Bugs Fixed * Fixed a rare race condition for `query plan` cache exceeding the allowed size limit - See [PR 31859](https://github.com/Azure/azure-sdk-for-java/pull/31859) * Added improvement in `RntbdClientChannelHealthChecker` for detecting continuous transit timeout. - See [PR 31544](https://github.com/Azure/azure-sdk-for-java/pull/31544) -* Fix an issue in replica validation where addresses sorting does not happen properly when replica validation is enabled. - See [PR 32022](https://github.com/Azure/azure-sdk-for-java/pull/32022) +* Fixed an issue in replica validation where addresses sorting does not happen properly when replica validation is enabled. - See [PR 32022](https://github.com/Azure/azure-sdk-for-java/pull/32022) #### Other Changes * Shaded `MurmurHash3` of apache `commons-codec` to enable removing of the `guava` dependency - CVE-2020-8908 - See [PR 31761](https://github.com/Azure/azure-sdk-for-java/pull/31761) From 2cebca23d1c4ad961bea3624aba8abf136fcf9d8 Mon Sep 17 00:00:00 2001 From: annie-mac Date: Tue, 8 Nov 2022 15:24:01 -0800 Subject: [PATCH 4/4] update changelog --- sdk/cosmos/azure-cosmos/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/cosmos/azure-cosmos/CHANGELOG.md b/sdk/cosmos/azure-cosmos/CHANGELOG.md index cf8853c4540a..a1b503d0fb08 100644 --- a/sdk/cosmos/azure-cosmos/CHANGELOG.md +++ b/sdk/cosmos/azure-cosmos/CHANGELOG.md @@ -9,7 +9,7 @@ #### Bugs Fixed * Fixed a rare race condition for `query plan` cache exceeding the allowed size limit - See [PR 31859](https://github.com/Azure/azure-sdk-for-java/pull/31859) * Added improvement in `RntbdClientChannelHealthChecker` for detecting continuous transit timeout. - See [PR 31544](https://github.com/Azure/azure-sdk-for-java/pull/31544) -* Fixed an issue in replica validation where addresses sorting does not happen properly when replica validation is enabled. - See [PR 32022](https://github.com/Azure/azure-sdk-for-java/pull/32022) +* Fixed an issue in replica validation where addresses may have not sorted properly when replica validation is enabled. - See [PR 32022](https://github.com/Azure/azure-sdk-for-java/pull/32022) #### Other Changes * Shaded `MurmurHash3` of apache `commons-codec` to enable removing of the `guava` dependency - CVE-2020-8908 - See [PR 31761](https://github.com/Azure/azure-sdk-for-java/pull/31761)