From 7961e3c3128965db73495d2d2295801299888d5d Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 12:00:43 +0900 Subject: [PATCH 01/10] test: Configures Multi-AZ Failover LoadBalancing Feature --- .../LoadBalancerClientConfigurationTests.java | 22 +++- ...overLoadBalancerCacheDataManagerTests.java | 89 ++++++++++++++ ...adBalancerLifecycleConfigurationTests.java | 50 ++++++++ ...iAZFailoverLoadBalancerLifecycleTests.java | 98 ++++++++++++++++ ...loverServiceInstanceListSupplierTests.java | 109 ++++++++++++++++++ 5 files changed, 366 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/cache/MultiAZFailoverLoadBalancerCacheDataManagerTests.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfigurationTests.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplierTests.java diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java index 3e26296e5..feebda586 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java @@ -26,10 +26,12 @@ import org.springframework.cloud.client.loadbalancer.LoadBalanced; import org.springframework.cloud.loadbalancer.config.LoadBalancerAutoConfiguration; import org.springframework.cloud.loadbalancer.config.LoadBalancerCacheAutoConfiguration; +import org.springframework.cloud.loadbalancer.config.LoadBalancerCacheDataManagerConfiguration; import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.DelegatingServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.DiscoveryClientServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.HealthCheckServiceInstanceListSupplier; +import org.springframework.cloud.loadbalancer.core.MultiAZFailoverServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.RequestBasedStickySessionServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.RetryAwareServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; @@ -48,13 +50,14 @@ * * @author Olga Maciaszek-Sharma * @author Zhuozhi Ji + * @author Jiwon Jeon */ class LoadBalancerClientConfigurationTests { ApplicationContextRunner reactiveDiscoveryClientRunner = new ApplicationContextRunner() .withConfiguration(AutoConfigurations.of(ReactiveCompositeDiscoveryClientAutoConfiguration.class, - LoadBalancerCacheAutoConfiguration.class, LoadBalancerAutoConfiguration.class, - LoadBalancerClientConfiguration.class)); + LoadBalancerCacheAutoConfiguration.class, LoadBalancerCacheDataManagerConfiguration.class, + LoadBalancerAutoConfiguration.class, LoadBalancerClientConfiguration.class)); ApplicationContextRunner blockingDiscoveryClientRunner = new ApplicationContextRunner() .withClassLoader(new FilteredClassLoader(RetryTemplate.class)) @@ -130,6 +133,21 @@ void shouldInstantiateWeightedServiceInstanceListSupplier() { }); } + @Test + void shouldInstantiateMultiAZFailoverServiceInstanceListSupplierTests() { + reactiveDiscoveryClientRunner.withPropertyValues("spring.cloud.loadbalancer.configurations=multi-az-failover") + .run(context -> { + ServiceInstanceListSupplier supplier = context.getBean(ServiceInstanceListSupplier.class); + then(supplier).isInstanceOf(CachingServiceInstanceListSupplier.class); + ServiceInstanceListSupplier delegate = ((DelegatingServiceInstanceListSupplier) supplier) + .getDelegate(); + then(delegate).isInstanceOf(MultiAZFailoverServiceInstanceListSupplier.class); + ServiceInstanceListSupplier secondDelegate = ((DelegatingServiceInstanceListSupplier) delegate) + .getDelegate(); + then(secondDelegate).isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class); + }); + } + @Test void shouldInstantiateRequestBasedStickySessionServiceInstanceListSupplierTests() { reactiveDiscoveryClientRunner.withUserConfiguration(TestConfig.class) diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/cache/MultiAZFailoverLoadBalancerCacheDataManagerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/cache/MultiAZFailoverLoadBalancerCacheDataManagerTests.java new file mode 100644 index 000000000..ab3b6800b --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/cache/MultiAZFailoverLoadBalancerCacheDataManagerTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.cache; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.cache.Cache; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.loadbalancer.core.MultiAZFailoverLoadBalancerCacheDataManager; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier.SERVICE_INSTANCE_CACHE_NAME; + +/** + * Tests for {@link MultiAZFailoverLoadBalancerCacheDataManager}. + * + * @author Jiwon Jeon + */ +class MultiAZFailoverLoadBalancerCacheDataManagerTests { + + private static final String CACHE_KEY_FAILED = "FAILED"; + + private final LoadBalancerCacheManager cacheManager = mock(LoadBalancerCacheManager.class); + + private final MultiAZFailoverLoadBalancerCacheDataManager cacheDataManager = new MultiAZFailoverLoadBalancerCacheDataManager( + cacheManager); + + @BeforeEach + void setUp() { + when(cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME)).thenReturn(mock(Cache.class)); + } + + @Test + void shouldReturnEmptySetWhenCacheIsEmpty() { + when(cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME).get(CACHE_KEY_FAILED, Set.class)) + .thenReturn(Collections.EMPTY_SET); + + assertThat(cacheDataManager.getInstance(CACHE_KEY_FAILED)).isEqualTo(Collections.EMPTY_SET); + } + + @Test + void shouldReturnFailedInstancesFromCache() { + final List failedInstances = List.of(new DefaultServiceInstance(), + new DefaultServiceInstance(), new DefaultServiceInstance()); + + when(cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME).get(CACHE_KEY_FAILED, Set.class)) + .thenReturn(new HashSet(failedInstances)); + + assertThat(cacheDataManager.getInstance(CACHE_KEY_FAILED)).isEqualTo(new HashSet<>(failedInstances)); + } + + @Test + void shouldAddFailedInstanceIntoCache() { + final ServiceInstance instance = new DefaultServiceInstance(); + final Set expected = new HashSet<>(List.of(instance)); + + when(cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME).get(CACHE_KEY_FAILED, Set.class)) + .thenReturn(Collections.EMPTY_SET); + when(cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME).get(CACHE_KEY_FAILED, Set.class)).thenReturn(expected); + + cacheDataManager.putInstance(instance); + + assertThat(cacheDataManager.getInstance(CACHE_KEY_FAILED)).isEqualTo(expected); + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfigurationTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfigurationTests.java new file mode 100644 index 000000000..26efb359f --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfigurationTests.java @@ -0,0 +1,50 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.config; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link LoadBalancerLifecycleConfiguration}. + * + * @author Jiwon Jeon + */ +class LoadBalancerLifecycleConfigurationTests { + + @Test + void shouldInstantiateMultiAZFailoverLoadBalancerLifecycle() { + baseApplicationRunner().withPropertyValues("spring.cloud.loadbalancer.configurations=multi-az-failover") + .run(context -> { + assertThat(context.getBeansOfType(LoadBalancerLifecycle.class)).hasSize(1); + assertThat(context.getBean("multiAZFailoverLoadBalancerLifecycle")) + .isInstanceOf(MultiAZFailoverLoadBalancerLifecycle.class); + }); + } + + private ApplicationContextRunner baseApplicationRunner() { + return new ApplicationContextRunner().withConfiguration( + AutoConfigurations.of(LoadBalancerAutoConfiguration.class, LoadBalancerCacheAutoConfiguration.class, + LoadBalancerCacheDataManagerConfiguration.class, LoadBalancerLifecycleConfiguration.class)); + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java new file mode 100644 index 000000000..9726a5bfc --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java @@ -0,0 +1,98 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.net.URI; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.DefaultRequest; +import org.springframework.cloud.client.loadbalancer.DefaultResponse; +import org.springframework.cloud.client.loadbalancer.Request; +import org.springframework.cloud.client.loadbalancer.RequestData; +import org.springframework.cloud.client.loadbalancer.ResponseData; +import org.springframework.cloud.client.loadbalancer.RetryableRequestContext; +import org.springframework.cloud.loadbalancer.config.MultiAZFailoverLoadBalancerLifecycle; +import org.springframework.http.HttpMethod; +import org.springframework.web.reactive.function.client.ClientRequest; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Tests for {@link MultiAZFailoverLoadBalancerLifecycle}. + * + * @author Jiwon Jeon + */ +class MultiAZFailoverLoadBalancerLifecycleTests { + + private final MultiAZFailoverLoadBalancerCacheDataManager cacheDataManager = mock( + MultiAZFailoverLoadBalancerCacheDataManager.class); + + private final MultiAZFailoverLoadBalancerLifecycle lifecycle = new MultiAZFailoverLoadBalancerLifecycle( + cacheDataManager); + + @Test + void shouldAnInstanceNotBeAddedIntoCache() { + final CompletionContext completionContext = new CompletionContext( + CompletionContext.Status.SUCCESS, getRequest(), new DefaultResponse(getServiceInstance())); + + lifecycle.onComplete(completionContext); + + verify(cacheDataManager, never()).putInstance(any(ServiceInstance.class)); + } + + @Test + void shouldClearCacheWhenNoInstancesAvailable() { + final CompletionContext completionContext = new CompletionContext( + CompletionContext.Status.DISCARD, getRequest(), new DefaultResponse(getServiceInstance())); + + lifecycle.onComplete(completionContext); + + verify(cacheDataManager, times(1)).clear(); + } + + @Test + void shouldAddInstanceIntoCacheWhenTheInstanceFailed() { + final CompletionContext completionContext = new CompletionContext( + CompletionContext.Status.FAILED, getRequest(), new DefaultResponse(getServiceInstance())); + + lifecycle.onComplete(completionContext); + + verify(cacheDataManager, times(1)).putInstance(any(ServiceInstance.class)); + } + + private Request getRequest() { + final var clientRequest = ClientRequest.create(HttpMethod.GET, URI.create("/test")).build(); + final var requestData = new RequestData(clientRequest); + final var requestContext = new RetryableRequestContext(null, requestData, "hint"); + + return new DefaultRequest<>(requestContext); + } + + private ServiceInstance getServiceInstance() { + return new DefaultServiceInstance(null, "test", "127.0.0.1", 8080, false, Map.of("zone", "zone1")); + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplierTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplierTests.java new file mode 100644 index 000000000..1b43dae0a --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplierTests.java @@ -0,0 +1,109 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; +import reactor.test.StepVerifier; + +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link MultiAZFailoverServiceInstanceListSupplier}. + * + * @author Jiwon Jeon + */ +class MultiAZFailoverServiceInstanceListSupplierTests { + + private static final String METADATA_KEY_ZONE = "zone"; + + private static final String CACHE_KEY_FAILED = "FAILED"; + + private final DiscoveryClientServiceInstanceListSupplier delegate = mock( + DiscoveryClientServiceInstanceListSupplier.class); + + private final LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig("zone1", List.of("zone2", "zone3")); + + private final ServiceInstance first = serviceInstance("test-1", buildZoneMetadata("zone1")); + + private final ServiceInstance second = serviceInstance("test-2", buildZoneMetadata("zone3")); + + private final ServiceInstance third = serviceInstance("test-3", buildZoneMetadata("zone2")); + + private final ServiceInstance fourth = serviceInstance("test-4", buildZoneMetadata("zone3")); + + private final ServiceInstance fifth = serviceInstance("test-5", buildZoneMetadata("zone1")); + + private final MultiAZFailoverLoadBalancerCacheDataManager cacheDataManager = mock( + MultiAZFailoverLoadBalancerCacheDataManager.class); + + private final MultiAZFailoverServiceInstanceListSupplier supplier = new MultiAZFailoverServiceInstanceListSupplier( + delegate, cacheDataManager, zoneConfig); + + @Test + void shouldFilterInstancesByZoneWhenNoAZFailed() { + when(cacheDataManager.getInstance(CACHE_KEY_FAILED, Set.class)).thenReturn(Collections.EMPTY_SET); + when(delegate.get()).thenReturn(Flux.just(List.of(first, second, third, fourth, fifth))); + + supplier.get().as(StepVerifier::create).assertNext(filtered -> { + assertThat(filtered).hasSize(5); + assertThat(filtered.get(0).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone1"); + assertThat(filtered.get(1).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone1"); + assertThat(filtered.get(2).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone2"); + assertThat(filtered.get(3).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone3"); + assertThat(filtered.get(4).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone3"); + }).verifyComplete(); + } + + @Test + void shouldFilterInstancesByZoneWhenAZFailed() { + when(cacheDataManager.getInstance(CACHE_KEY_FAILED, Set.class)) + .thenReturn(new HashSet<>(List.of(first, fifth))); + when(delegate.get()).thenReturn(Flux.just(List.of(first, second, third, fourth, fifth))); + + supplier.get().as(StepVerifier::create).assertNext(filtered -> { + assertThat(filtered).hasSize(3); + assertThat(filtered.get(0).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone2"); + assertThat(filtered.get(1).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone3"); + assertThat(filtered.get(2).getMetadata().get(METADATA_KEY_ZONE)).isEqualTo("zone3"); + }).verifyComplete(); + } + + private DefaultServiceInstance serviceInstance(String instanceId, Map metadata) { + return new DefaultServiceInstance(instanceId, "test", "http://test.test", 9080, false, metadata); + } + + private Map buildZoneMetadata(String zone) { + Map metadata = new HashMap<>(); + metadata.put(METADATA_KEY_ZONE, zone); + return metadata; + } + +} From e4b63f57d00ec43b754a30de6cc8fd23fb2f320e Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 12:00:52 +0900 Subject: [PATCH 02/10] feat: Configures Multi-AZ Failover LoadBalancing Feature --- .../LoadBalancerClientConfiguration.java | 30 +++++++ ...BalancerCacheDataManagerConfiguration.java | 84 +++++++++++++++++++ .../LoadBalancerLifecycleConfiguration.java | 44 ++++++++++ .../config/LoadBalancerZoneConfig.java | 21 ++++- .../MultiAZFailoverLoadBalancerLifecycle.java | 84 +++++++++++++++++++ .../core/LoadBalancerCacheDataManager.java | 39 +++++++++ ...ZFailoverLoadBalancerCacheDataManager.java | 74 ++++++++++++++++ ...AZFailoverServiceInstanceListSupplier.java | 71 ++++++++++++++++ .../ServiceInstanceListSupplierBuilder.java | 19 +++++ 9 files changed, 465 insertions(+), 1 deletion(-) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerCacheDataManagerConfiguration.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfiguration.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/LoadBalancerCacheDataManager.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerCacheDataManager.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplier.java diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 8f71163b9..07191c310 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -114,6 +114,16 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList return ServiceInstanceListSupplier.builder().withDiscoveryClient().withHealthChecks().build(context); } + @Bean + @ConditionalOnBean(ReactiveDiscoveryClient.class) + @ConditionalOnMissingBean + @Conditional(MultiAZFailoverConfigurationCondition.class) + public ServiceInstanceListSupplier multiAZFailoverDiscoveryClientServiceInstanceListSupplier( + ConfigurableApplicationContext context) { + return ServiceInstanceListSupplier.builder().withDiscoveryClient().withMultiAZFailover().withCaching() + .build(context); + } + @Bean @ConditionalOnBean(ReactiveDiscoveryClient.class) @ConditionalOnMissingBean @@ -179,6 +189,16 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList .build(context); } + @Bean + @ConditionalOnBean(DiscoveryClient.class) + @ConditionalOnMissingBean + @Conditional(MultiAZFailoverConfigurationCondition.class) + public ServiceInstanceListSupplier multiAZFailoverServiceInstanceListSupplier( + ConfigurableApplicationContext context) { + return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withMultiAZFailover() + .withCaching().build(context); + } + @Bean @ConditionalOnBean(DiscoveryClient.class) @ConditionalOnMissingBean @@ -323,6 +343,16 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) } + static class MultiAZFailoverConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(context.getEnvironment(), + "configurations", "multi-az-failover"); + } + + } + static class RequestBasedStickySessionConfigurationCondition implements Condition { @Override diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerCacheDataManagerConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerCacheDataManagerConfiguration.java new file mode 100644 index 000000000..fb6bfb68b --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerCacheDataManagerConfiguration.java @@ -0,0 +1,84 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.config; + +import com.github.benmanes.caffeine.cache.Caffeine; + +import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.AutoConfigureBefore; +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cache.caffeine.CaffeineCacheManager; +import org.springframework.cloud.loadbalancer.cache.LoadBalancerCacheManager; +import org.springframework.cloud.loadbalancer.core.LoadBalancerCacheDataManager; +import org.springframework.cloud.loadbalancer.core.MultiAZFailoverLoadBalancerCacheDataManager; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; + +/** + * An AutoConfiguration that provides a {@link LoadBalancerCacheDataManager} bean for + * adding ServiceInstance into a cache. + * + * @author Jiwon Jeon + */ +@Configuration(proxyBeanMethods = false) +@AutoConfigureAfter(LoadBalancerCacheAutoConfiguration.class) +@AutoConfigureBefore(LoadBalancerLifecycleConfiguration.class) +public class LoadBalancerCacheDataManagerConfiguration { + + @Bean + @DependsOn("caffeineLoadBalancerCacheManager") + @ConditionalOnClass({ Caffeine.class, CaffeineCacheManager.class }) + @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "multi-az-failover") + LoadBalancerCacheDataManager multiAZFailoverCaffeineLoadBalancerCacheDataManager(ApplicationContext context) { + return new MultiAZFailoverLoadBalancerCacheDataManager( + context.getBean("caffeineLoadBalancerCacheManager", LoadBalancerCacheManager.class)); + } + + @Bean + @DependsOn("defaultLoadBalancerCacheManager") + @Conditional(OnCaffeineCacheMissingCondition.class) + @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "multi-az-failover") + LoadBalancerCacheDataManager multiAZFailoverDefaultLoadBalancerCacheDataManager(ApplicationContext context) { + return new MultiAZFailoverLoadBalancerCacheDataManager( + context.getBean("defaultLoadBalancerCacheManager", LoadBalancerCacheManager.class)); + } + + static final class OnCaffeineCacheMissingCondition extends AnyNestedCondition { + + private OnCaffeineCacheMissingCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnMissingClass("com.github.benmanes.caffeine.cache.Caffeine") + static class CaffeineClassMissing { + + } + + @ConditionalOnMissingClass("org.springframework.cache.caffeine.CaffeineCacheManager") + static class CaffeineCacheManagerClassMissing { + + } + + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfiguration.java new file mode 100644 index 000000000..7a2c518d6 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerLifecycleConfiguration.java @@ -0,0 +1,44 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.config; + +import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; +import org.springframework.cloud.loadbalancer.core.LoadBalancerCacheDataManager; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * An AutoConfiguration that provides {@link LoadBalancerLifecycle} bean defining + * behaviors before and after load-balancing. + * + * @author Jiwon Jeon + */ +@Configuration(proxyBeanMethods = false) +@AutoConfigureAfter({ LoadBalancerCacheAutoConfiguration.class, LoadBalancerCacheDataManagerConfiguration.class }) +public class LoadBalancerLifecycleConfiguration { + + @Bean + @ConditionalOnBean(LoadBalancerCacheDataManager.class) + @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "multi-az-failover") + LoadBalancerLifecycle multiAZFailoverLoadBalancerLifecycle(LoadBalancerCacheDataManager cacheDataManager) { + return new MultiAZFailoverLoadBalancerLifecycle(cacheDataManager); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java index 9809b4d1c..c72337543 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java @@ -16,8 +16,12 @@ package org.springframework.cloud.loadbalancer.config; +import java.util.Collections; +import java.util.List; + /** * @author Olga Maciaszek-Sharma + * @author Jiwon Jeon */ public class LoadBalancerZoneConfig { @@ -27,8 +31,15 @@ public class LoadBalancerZoneConfig { */ private String zone; + private List secondaryZones; + public LoadBalancerZoneConfig(String zone) { - this.zone = zone; + this(zone, Collections.EMPTY_LIST); + } + + public LoadBalancerZoneConfig(String primaryZone, List secondaryZones) { + this.zone = primaryZone; + this.secondaryZones = secondaryZones; } public String getZone() { @@ -39,4 +50,12 @@ public void setZone(String zone) { this.zone = zone; } + public List getSecondaryZones() { + return secondaryZones; + } + + public void setSecondaryZones(List secondaryZones) { + this.secondaryZones = secondaryZones; + } + } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java new file mode 100644 index 000000000..f66051055 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java @@ -0,0 +1,84 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.config; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; +import org.springframework.cloud.client.loadbalancer.Request; +import org.springframework.cloud.client.loadbalancer.Response; +import org.springframework.cloud.client.loadbalancer.ResponseData; +import org.springframework.cloud.loadbalancer.core.LoadBalancerCacheDataManager; +import org.springframework.web.reactive.result.view.RequestContext; + +/** + * A LoadBalancerLifecycle implementation that defines behaviors before and after + * load-balancing, considering Multi-AZ failover. + * + * @author Jiwon Jeon + */ +public class MultiAZFailoverLoadBalancerLifecycle + implements LoadBalancerLifecycle { + + private static final Log LOG = LogFactory.getLog(MultiAZFailoverLoadBalancerLifecycle.class); + + private final LoadBalancerCacheDataManager cacheDataManager; + + public MultiAZFailoverLoadBalancerLifecycle(LoadBalancerCacheDataManager cacheDataManager) { + this.cacheDataManager = cacheDataManager; + } + + @Override + public boolean supports(Class requestContextClass, Class responseClass, Class serverTypeClass) { + return RequestContext.class.isAssignableFrom(requestContextClass) + && ResponseData.class.isAssignableFrom(responseClass) + && ServiceInstance.class.isAssignableFrom(serverTypeClass); + } + + @Override + public void onStart(Request request) { + } + + @Override + public void onStartRequest(Request request, Response lbResponse) { + } + + @Override + public void onComplete(CompletionContext completionContext) { + final CompletionContext.Status status = completionContext.status(); + final ServiceInstance instance = completionContext.getLoadBalancerResponse().getServer(); + + if (CompletionContext.Status.DISCARD.equals(status)) { + if (LOG.isWarnEnabled()) { + LOG.warn("Any instance selected. Cache will be evicted..."); + } + + cacheDataManager.clear(); + } + else if (CompletionContext.Status.FAILED.equals(status)) { + if (LOG.isErrorEnabled()) { + LOG.error(String.format("Requesting to [%s] failed", instance)); + } + + cacheDataManager.putInstance(instance); + } + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/LoadBalancerCacheDataManager.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/LoadBalancerCacheDataManager.java new file mode 100644 index 000000000..0f3513ca2 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/LoadBalancerCacheDataManager.java @@ -0,0 +1,39 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import org.springframework.cache.Cache; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.loadbalancer.cache.LoadBalancerCacheManager; + +/** + * An interface allows adding ServiceInstance into cache through + * {@link LoadBalancerCacheManager}. + * + * @author Jiwon Jeon + */ +public interface LoadBalancerCacheDataManager { + + Cache getCache(); + + void clear(); + + T getInstance(String key, Class classType); + + void putInstance(ServiceInstance instance); + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerCacheDataManager.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerCacheDataManager.java new file mode 100644 index 000000000..3bcb20200 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerCacheDataManager.java @@ -0,0 +1,74 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.HashSet; +import java.util.Set; + +import org.springframework.cache.Cache; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.loadbalancer.cache.LoadBalancerCacheManager; + +import static org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier.SERVICE_INSTANCE_CACHE_NAME; + +/** + * A CacheDataManager implementation that logs connection-failed ServiceInstances. + * + * @author Jiwon Jeon + */ +public class MultiAZFailoverLoadBalancerCacheDataManager implements LoadBalancerCacheDataManager { + + private static final String CACHE_KEY_FAILED = "FAILED"; + + private final LoadBalancerCacheManager cacheManager; + + public MultiAZFailoverLoadBalancerCacheDataManager(LoadBalancerCacheManager cacheManager) { + this.cacheManager = cacheManager; + } + + @Override + public Cache getCache() { + return cacheManager.getCache(SERVICE_INSTANCE_CACHE_NAME); + } + + @Override + public void clear() { + getCache().clear(); + } + + @Override + public T getInstance(String key, Class classType) { + return getCache().get(key, classType); + } + + @Override + public void putInstance(ServiceInstance instance) { + Set failedInstances = getInstance(CACHE_KEY_FAILED); + + if (failedInstances == null) { + failedInstances = new HashSet<>(); + } + + failedInstances.add(instance); + getCache().put(CACHE_KEY_FAILED, failedInstances); + } + + public Set getInstance(String key) { + return getInstance(key, Set.class); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplier.java new file mode 100644 index 000000000..3f091150b --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverServiceInstanceListSupplier.java @@ -0,0 +1,71 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; + +import reactor.core.publisher.Flux; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig; + +import static java.util.Comparator.comparing; + +/** + * A Multi-AZ Failover implementation of {@link ServiceInstanceListSupplier}. + * + * @author Jiwon Jeon + */ +public class MultiAZFailoverServiceInstanceListSupplier extends DelegatingServiceInstanceListSupplier { + + private static final String CACHE_KEY_FAILED = "FAILED"; + + private static final String METADATA_KEY_ZONE = "zone"; + + private final LoadBalancerCacheDataManager cacheDataManager; + + private final LoadBalancerZoneConfig zoneConfig; + + public MultiAZFailoverServiceInstanceListSupplier(ServiceInstanceListSupplier delegate, + LoadBalancerCacheDataManager cacheDataManager, LoadBalancerZoneConfig zoneConfig) { + super(delegate); + this.cacheDataManager = cacheDataManager; + this.zoneConfig = zoneConfig; + } + + @Override + public Flux> get() { + return getDelegate().get().map(this::filteredAndOrderedByZone); + } + + private List filteredAndOrderedByZone(List serviceInstances) { + final Set instances = cacheDataManager.getInstance(CACHE_KEY_FAILED, Set.class); + final List zones = Stream + .concat(Stream.of(zoneConfig.getZone()), zoneConfig.getSecondaryZones().stream()).toList(); + final Stream instanceStream = serviceInstances.stream() + .sorted(comparing(instance -> zones.indexOf(instance.getMetadata().get(METADATA_KEY_ZONE)))); + + if (instances == null) { + return instanceStream.toList(); + } + + return instanceStream.filter(instance -> !instances.contains(instance)).toList(); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java index d0a8ecdc9..01dd197e9 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java @@ -51,6 +51,7 @@ * @author Zhiguo Chen * @author Sabyasachi Bhattacharya * @author Zhuozhi Ji + * @author Jiwon Jeon */ public final class ServiceInstanceListSupplierBuilder { @@ -239,6 +240,24 @@ public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) { return this; } + /** + * Adds a {@link MultiAZFailoverServiceInstanceListSupplier} to the + * {@link ServiceInstanceListSupplier} hierarchy. + * @return the {@link ServiceInstanceListSupplierBuilder} object + */ + public ServiceInstanceListSupplierBuilder withMultiAZFailover() { + DelegateCreator creator = (context, delegate) -> { + LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class); + LoadBalancerCacheManager cacheManager = context.getBean(LoadBalancerCacheManager.class); + LoadBalancerCacheDataManager cacheDataManager = new MultiAZFailoverLoadBalancerCacheDataManager( + cacheManager); + + return new MultiAZFailoverServiceInstanceListSupplier(delegate, cacheDataManager, zoneConfig); + }; + this.creators.add(creator); + return this; + } + /** * Adds a {@link RequestBasedStickySessionServiceInstanceListSupplier} to the * {@link ServiceInstanceListSupplier} hierarchy. From 49d90ae1e31604d2d34442470a01b041d916c34d Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 12:07:27 +0900 Subject: [PATCH 03/10] feat: Disable Multi-AZ Failover feature when cannot use Caching Feature --- .../core/ServiceInstanceListSupplierBuilder.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java index 01dd197e9..3f237efb8 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java @@ -248,7 +248,15 @@ public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) { public ServiceInstanceListSupplierBuilder withMultiAZFailover() { DelegateCreator creator = (context, delegate) -> { LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class); - LoadBalancerCacheManager cacheManager = context.getBean(LoadBalancerCacheManager.class); + ObjectProvider cacheManagerProvider = context.getBeanProvider(LoadBalancerCacheManager.class); + if (cacheManagerProvider.getIfAvailable() == null) { + if (LOG.isWarnEnabled()) { + LOG.warn("Cannot enable caching so that Multi-AZ Failover will be disabled."); + } + return delegate; + } + + LoadBalancerCacheManager cacheManager = cacheManagerProvider.getIfAvailable(); LoadBalancerCacheDataManager cacheDataManager = new MultiAZFailoverLoadBalancerCacheDataManager( cacheManager); From 951ff0ca0e7637f63f38bc52bcc7ad3b3e724af4 Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 12:16:56 +0900 Subject: [PATCH 04/10] feat: New property spring.cloud.loadbalancer.secondary-zones --- .../config/LoadBalancerAutoConfiguration.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java index 5d0bab627..e643c71f6 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java @@ -18,6 +18,9 @@ import java.util.Collections; import java.util.List; +import java.util.Optional; + +import io.netty.util.internal.StringUtil; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfigureBefore; @@ -41,6 +44,7 @@ /** * @author Spencer Gibb * @author Olga Maciaszek-Sharma + * @author Jiwon Jeon */ @Configuration(proxyBeanMethods = false) @LoadBalancerClients @@ -50,10 +54,14 @@ @ConditionalOnProperty(value = "spring.cloud.loadbalancer.enabled", havingValue = "true", matchIfMissing = true) public class LoadBalancerAutoConfiguration { + private static final String ZONE_SPLITTER_COMMA = ","; + @Bean @ConditionalOnMissingBean public LoadBalancerZoneConfig zoneConfig(Environment environment) { - return new LoadBalancerZoneConfig(environment.getProperty("spring.cloud.loadbalancer.zone")); + return new LoadBalancerZoneConfig(environment.getProperty("spring.cloud.loadbalancer.zone"), + List.of(Optional.ofNullable(environment.getProperty("spring.cloud.loadbalancer.secondary-zones")) + .orElse(StringUtil.EMPTY_STRING).split(ZONE_SPLITTER_COMMA))); } @ConditionalOnMissingBean From fd5a363e98efc05b02dca6b1448aecf3070f3ce9 Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 12:17:07 +0900 Subject: [PATCH 05/10] style: New line --- .../loadbalancer/core/ServiceInstanceListSupplierBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java index 3f237efb8..221c636ac 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java @@ -248,7 +248,8 @@ public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) { public ServiceInstanceListSupplierBuilder withMultiAZFailover() { DelegateCreator creator = (context, delegate) -> { LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class); - ObjectProvider cacheManagerProvider = context.getBeanProvider(LoadBalancerCacheManager.class); + ObjectProvider cacheManagerProvider = context + .getBeanProvider(LoadBalancerCacheManager.class); if (cacheManagerProvider.getIfAvailable() == null) { if (LOG.isWarnEnabled()) { LOG.warn("Cannot enable caching so that Multi-AZ Failover will be disabled."); From 30280549958d7c9cf52460574c66269665009dac Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 14:32:34 +0900 Subject: [PATCH 06/10] feat: Removes Optional / StringUtil --- .../config/LoadBalancerAutoConfiguration.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java index e643c71f6..711749828 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java @@ -18,9 +18,6 @@ import java.util.Collections; import java.util.List; -import java.util.Optional; - -import io.netty.util.internal.StringUtil; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfigureBefore; @@ -48,9 +45,9 @@ */ @Configuration(proxyBeanMethods = false) @LoadBalancerClients -@EnableConfigurationProperties({ LoadBalancerClientsProperties.class, LoadBalancerEagerLoadProperties.class }) -@AutoConfigureBefore({ ReactorLoadBalancerClientAutoConfiguration.class, - LoadBalancerBeanPostProcessorAutoConfiguration.class }) +@EnableConfigurationProperties({LoadBalancerClientsProperties.class, LoadBalancerEagerLoadProperties.class}) +@AutoConfigureBefore({ReactorLoadBalancerClientAutoConfiguration.class, + LoadBalancerBeanPostProcessorAutoConfiguration.class}) @ConditionalOnProperty(value = "spring.cloud.loadbalancer.enabled", havingValue = "true", matchIfMissing = true) public class LoadBalancerAutoConfiguration { @@ -60,8 +57,8 @@ public class LoadBalancerAutoConfiguration { @ConditionalOnMissingBean public LoadBalancerZoneConfig zoneConfig(Environment environment) { return new LoadBalancerZoneConfig(environment.getProperty("spring.cloud.loadbalancer.zone"), - List.of(Optional.ofNullable(environment.getProperty("spring.cloud.loadbalancer.secondary-zones")) - .orElse(StringUtil.EMPTY_STRING).split(ZONE_SPLITTER_COMMA))); + List.of(environment.getProperty("spring.cloud.loadbalancer.secondary-zones", "") + .split(ZONE_SPLITTER_COMMA))); } @ConditionalOnMissingBean From 481305cd3102f9b905a30beaecc0624545e02bb1 Mon Sep 17 00:00:00 2001 From: G1 Date: Tue, 7 Mar 2023 14:40:02 +0900 Subject: [PATCH 07/10] feat: Uses StringUtils.commaDelimitedListToStringArray instead --- .../config/LoadBalancerAutoConfiguration.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java index 711749828..1228e29b3 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.cloud.loadbalancer.config; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -37,6 +38,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.env.Environment; +import org.springframework.util.StringUtils; /** * @author Spencer Gibb @@ -51,14 +53,12 @@ @ConditionalOnProperty(value = "spring.cloud.loadbalancer.enabled", havingValue = "true", matchIfMissing = true) public class LoadBalancerAutoConfiguration { - private static final String ZONE_SPLITTER_COMMA = ","; - @Bean @ConditionalOnMissingBean public LoadBalancerZoneConfig zoneConfig(Environment environment) { return new LoadBalancerZoneConfig(environment.getProperty("spring.cloud.loadbalancer.zone"), - List.of(environment.getProperty("spring.cloud.loadbalancer.secondary-zones", "") - .split(ZONE_SPLITTER_COMMA))); + Arrays.stream(StringUtils.commaDelimitedListToStringArray(environment.getProperty("spring.cloud.loadbalancer.secondary-zones", ""))) + .toList()); } @ConditionalOnMissingBean From 43b4d03bd6223d38505d5880dbdba172e01bb9df Mon Sep 17 00:00:00 2001 From: G1 Date: Sun, 19 Mar 2023 13:32:27 +0900 Subject: [PATCH 08/10] feat: Failed Instances only when connection failed or 5xx error will be added into cache --- .../MultiAZFailoverLoadBalancerLifecycle.java | 4 +- ...iAZFailoverLoadBalancerLifecycleTests.java | 40 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java index f66051055..d76254166 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/MultiAZFailoverLoadBalancerLifecycle.java @@ -64,6 +64,7 @@ public void onStartRequest(Request request, Response completionContext) { final CompletionContext.Status status = completionContext.status(); final ServiceInstance instance = completionContext.getLoadBalancerResponse().getServer(); + final ResponseData clientResponse = completionContext.getClientResponse(); if (CompletionContext.Status.DISCARD.equals(status)) { if (LOG.isWarnEnabled()) { @@ -72,7 +73,8 @@ public void onComplete(CompletionContext( CompletionContext.Status.FAILED, getRequest(), new DefaultResponse(getServiceInstance())); lifecycle.onComplete(completionContext); + assertNull(completionContext.getClientResponse()); + + verify(cacheDataManager, times(1)).putInstance(any(ServiceInstance.class)); + } + + @Test + void shouldNotAddInstanceIntoCacheWhenHeartbeat404() { + final var request = getRequest(); + final CompletionContext completionContext = new CompletionContext( + CompletionContext.Status.FAILED, + request, + new DefaultResponse(getServiceInstance()), + new ResponseData(HttpStatus.NOT_FOUND, new HttpHeaders(), new LinkedMultiValueMap<>(), request.getContext() + .getClientRequest()) + ); + + lifecycle.onComplete(completionContext); + + verify(cacheDataManager, never()).putInstance(any(ServiceInstance.class)); + } + + @Test + void shouldAddInstanceIntoCacheWhen5xxErrorOccurred() { + final var request = getRequest(); + final CompletionContext completionContext = new CompletionContext( + CompletionContext.Status.FAILED, + request, + new DefaultResponse(getServiceInstance()), + new ResponseData(HttpStatus.BAD_GATEWAY, new HttpHeaders(), new LinkedMultiValueMap<>(), request.getContext() + .getClientRequest()) + ); + + lifecycle.onComplete(completionContext); + verify(cacheDataManager, times(1)).putInstance(any(ServiceInstance.class)); } From 3581edf95aa34e3615faed9f4fb240b48f910fd5 Mon Sep 17 00:00:00 2001 From: G1 Date: Sun, 19 Mar 2023 13:32:27 +0900 Subject: [PATCH 09/10] chore: Rename Test method --- .../core/MultiAZFailoverLoadBalancerLifecycleTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java index 05dedccf5..24bfbfce3 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java @@ -90,7 +90,7 @@ void shouldAddInstanceIntoCacheWhenTheInstanceConnectionFailed() { } @Test - void shouldNotAddInstanceIntoCacheWhenHeartbeat404() { + void shouldNotAddInstanceIntoCacheWhen4xxError() { final var request = getRequest(); final CompletionContext completionContext = new CompletionContext( CompletionContext.Status.FAILED, From 0d6d1e44a56937ff0ff9e11ca30cc39821358b21 Mon Sep 17 00:00:00 2001 From: G1 Date: Sat, 15 Apr 2023 17:24:06 +0900 Subject: [PATCH 10/10] test: Fixes Build Error - assertNull -> assertThat.isNull --- .../core/MultiAZFailoverLoadBalancerLifecycleTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java index 24bfbfce3..44ca44254 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/MultiAZFailoverLoadBalancerLifecycleTests.java @@ -37,7 +37,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.web.reactive.function.client.ClientRequest; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -84,7 +84,7 @@ void shouldAddInstanceIntoCacheWhenTheInstanceConnectionFailed() { lifecycle.onComplete(completionContext); - assertNull(completionContext.getClientResponse()); + assertThat(completionContext.getClientResponse()).isNull(); verify(cacheDataManager, times(1)).putInstance(any(ServiceInstance.class)); }