This is an automated email from the ASF dual-hosted git repository. liubao pushed a commit to branch 1.3.x in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit 3f5ed630a49d6e08556d4d7933560225deb5a767 Author: yhs0092 <[email protected]> AuthorDate: Tue Sep 1 22:45:30 2020 +0800 [SCB-2077] fix instance isolation trying chance not released problem cherry-pick from master branch and fix conflicts and problems --- .../servicecomb/foundation/common/Holder.java | 41 +++++++++++++++ .../foundation/common/testing/MockClock.java | 53 +++++++++++++++++++ .../foundation/common/utils/TimeUtils.java | 33 ++++++++++++ .../servicecomb/loadbalance/Configuration.java | 18 ++++++- .../loadbalance/LoadbalanceHandler.java | 5 +- .../loadbalance/ServiceCombServerStats.java | 61 ++++++++++++++++++---- .../loadbalance/TryingIsolatedServerMarker.java | 56 ++++++++++++++++++++ .../filter/IsolationDiscoveryFilter.java | 8 +-- .../servicecomb/loadbalance/TestConfiguration.java | 15 ++++++ .../loadbalance/TestLoadBalanceHandler2.java | 8 +-- .../loadbalance/TestServiceCombServerStats.java | 59 +++++++++++++++++++++ .../filter/IsolationDiscoveryFilterTest.java | 14 ++--- 12 files changed, 337 insertions(+), 34 deletions(-) diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/Holder.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/Holder.java new file mode 100644 index 0000000..ebc92a3 --- /dev/null +++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/Holder.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.servicecomb.foundation.common; + +// do not use javax.xml.ws.Holder, use this one. Because JDK 11 above do not have javax.ws.Holder +public final class Holder<T> { + + /** + * The value contained in the holder. + */ + public T value; + + /** + * Creates a new holder with a <code>null</code> value. + */ + public Holder() { + } + + /** + * Create a new holder with the specified value. + * + * @param value The value to be stored in the holder. + */ + public Holder(T value) { + this.value = value; + } +} diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/testing/MockClock.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/testing/MockClock.java new file mode 100644 index 0000000..252e64b --- /dev/null +++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/testing/MockClock.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.servicecomb.foundation.common.testing; + +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; + +import org.apache.servicecomb.foundation.common.Holder; + +public class MockClock extends Clock { + + Holder<Long> mockMillsHolder; + + public MockClock(Holder<Long> h) { + this.mockMillsHolder = h; + } + + @Override + public ZoneId getZone() { + return null; + } + + @Override + public Clock withZone(ZoneId zone) { + return null; + } + + @Override + public Instant instant() { + return null; + } + + @Override + public long millis() { + return mockMillsHolder.value; + } +} diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/TimeUtils.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/TimeUtils.java new file mode 100644 index 0000000..245a4ee --- /dev/null +++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/TimeUtils.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.servicecomb.foundation.common.utils; + +import java.time.Clock; + +public final class TimeUtils { + private TimeUtils() { + } + + private static final Clock systemDefaultZoneClock = Clock.systemDefaultZone(); + + /** + * @return Singleton system default zone clock + */ + public static Clock getSystemDefaultZoneClock() { + return systemDefaultZoneClock; + } +} diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java index fd3190f..905d09d 100644 --- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java +++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java @@ -66,6 +66,8 @@ public final class Configuration { public static final String FILTER_SINGLE_TEST = "singleTestTime"; + public static final String FILTER_MAX_SINGLE_TEST_WINDOW = "maxSingleTestWindow"; + public static final String FILTER_MIN_ISOLATION_TIME = "minIsolationTime"; public static final String FILTER_CONTINUOUS_FAILURE_THRESHOLD = "continuousFailureThreshold"; @@ -203,7 +205,21 @@ public final class Configuration { return result; } return defaultValue; + } catch (NumberFormatException e) { + return defaultValue; + } + } + public int getMaxSingleTestWindow() { + final int defaultValue = 60000; + String p = getStringProperty(Integer.toString(defaultValue), + PROP_ROOT + FILTER_ISOLATION + FILTER_MAX_SINGLE_TEST_WINDOW); + try { + int result = Integer.parseInt(p); + if (result >= 0) { + return result; + } + return defaultValue; } catch (NumberFormatException e) { return defaultValue; } @@ -231,7 +247,7 @@ public final class Configuration { } public static String getStringProperty(String defaultValue, String... keys) { - String property = null; + String property; for (String key : keys) { property = DynamicPropertyFactory.getInstance().getStringProperty(key, null).get(); if (property != null) { diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java index 00155cb..00be81d 100644 --- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java +++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java @@ -40,7 +40,6 @@ import org.apache.servicecomb.core.provider.consumer.SyncResponseExecutor; import org.apache.servicecomb.foundation.common.cache.VersionedCache; import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx; import org.apache.servicecomb.foundation.common.utils.ExceptionUtils; -import org.apache.servicecomb.loadbalance.filter.IsolationDiscoveryFilter; import org.apache.servicecomb.loadbalance.filter.ServerDiscoveryFilter; import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext; import org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter; @@ -193,9 +192,7 @@ public class LoadbalanceHandler implements Handler { public void handle(Invocation invocation, AsyncResponse asyncResp) throws Exception { AsyncResponse response = asyncResp; asyncResp = async -> { - if (Boolean.TRUE.equals(invocation.getLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING))) { - ServiceCombServerStats.releaseTryingChance(); - } + ServiceCombServerStats.checkAndReleaseTryingChance(invocation); response.handle(async); }; if (supportDefinedEndpoint) { diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java index 5c50c74..5ad7624 100644 --- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java +++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java @@ -17,9 +17,12 @@ package org.apache.servicecomb.loadbalance; -import java.util.concurrent.atomic.AtomicBoolean; +import java.time.Clock; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.common.utils.TimeUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,17 +36,18 @@ public class ServiceCombServerStats { private static final Logger LOGGER = LoggerFactory.getLogger(ServiceCombServerStats.class); + private final Object lock = new Object(); + /** * There is not more than 1 server allowed to stay in TRYING status concurrently. - * If the value of globalAllowIsolatedServerTryingFlag is false, there have been 1 server in - * TRYING status, so the other isolated servers cannot be transferred into - * TRYING status; otherwise the value of globalAllowIsolatedServerTryingFlag is true. + * This flag is designed to ensure such mechanism. And it makes the ServiceCombServerStats stateful. + * Therefore, the flag should be reset correctly after the trying server gets handled. */ - private static AtomicBoolean globalAllowIsolatedServerTryingFlag = new AtomicBoolean(true); + static AtomicReference<TryingIsolatedServerMarker> globalAllowIsolatedServerTryingFlag = new AtomicReference<>(); private long lastWindow = System.currentTimeMillis(); - private final Object lock = new Object(); + Clock clock; private AtomicLong continuousFailureCount = new AtomicLong(0); @@ -59,8 +63,32 @@ public class ServiceCombServerStats { private boolean isolated = false; + public ServiceCombServerStats() { + this.clock = TimeUtils.getSystemDefaultZoneClock(); + init(); + } + + public ServiceCombServerStats(Clock clock) { + this.clock = clock; + init(); + } + + private void init() { + lastWindow = clock.millis(); + continuousFailureCount = new AtomicLong(0); + lastVisitTime = clock.millis(); + lastActiveTime = clock.millis(); + totalRequests = new AtomicLong(0L); + successRequests = new AtomicLong(0L); + failedRequests = new AtomicLong(0L); + } + public static boolean isolatedServerCanTry() { - return globalAllowIsolatedServerTryingFlag.get(); + TryingIsolatedServerMarker marker = globalAllowIsolatedServerTryingFlag.get(); + if (marker == null) { + return true; + } + return marker.isOutdated(); } /** @@ -68,12 +96,23 @@ public class ServiceCombServerStats { * * @return true if the chance is applied successfully, otherwise false */ - public static boolean applyForTryingChance() { - return isolatedServerCanTry() && globalAllowIsolatedServerTryingFlag.compareAndSet(true, false); + public static boolean applyForTryingChance(Invocation invocation) { + TryingIsolatedServerMarker marker = globalAllowIsolatedServerTryingFlag.get(); + if (marker == null) { + return globalAllowIsolatedServerTryingFlag.compareAndSet(null, new TryingIsolatedServerMarker(invocation)); + } + if (marker.isOutdated()) { + return globalAllowIsolatedServerTryingFlag.compareAndSet(marker, new TryingIsolatedServerMarker(invocation)); + } + return false; } - public static void releaseTryingChance() { - globalAllowIsolatedServerTryingFlag.set(true); + public static void checkAndReleaseTryingChance(Invocation invocation) { + TryingIsolatedServerMarker marker = globalAllowIsolatedServerTryingFlag.get(); + if (marker == null || marker.getInvocation() != invocation) { + return; + } + globalAllowIsolatedServerTryingFlag.compareAndSet(marker, null); } public void markIsolated(boolean isolated) { diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/TryingIsolatedServerMarker.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/TryingIsolatedServerMarker.java new file mode 100644 index 0000000..a741a8d --- /dev/null +++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/TryingIsolatedServerMarker.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.servicecomb.loadbalance; + +import java.time.Clock; +import java.util.Objects; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.common.utils.TimeUtils; + +public class TryingIsolatedServerMarker { + Clock clock; + + final long startTryingTimestamp; + + private final Invocation invocation; + + /** + * To make sure even if some error interrupted the releasing operation of + * {@link ServiceCombServerStats#globalAllowIsolatedServerTryingFlag}, + * the state can still be recovered. + */ + private final int maxSingleTestWindow; + + public TryingIsolatedServerMarker(Invocation invocation) { + Objects.requireNonNull(invocation, "invocation should be non-null"); + this.invocation = invocation; + clock = TimeUtils.getSystemDefaultZoneClock(); + startTryingTimestamp = clock.millis(); + maxSingleTestWindow = Configuration.INSTANCE.getMaxSingleTestWindow(); + } + + public boolean isOutdated() { + return this.invocation.isFinished() + || clock.millis() - startTryingTimestamp >= maxSingleTestWindow; + } + + public Invocation getInvocation() { + return invocation; + } +} diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java index 67a46cb..d6feeac 100644 --- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java +++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java @@ -144,13 +144,7 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter { if (!checkThresholdAllowed(settings, serverStats)) { if (serverStats.isIsolated() && (System.currentTimeMillis() - serverStats.getLastVisitTime()) > settings.singleTestTime) { - if (!ServiceCombServerStats.applyForTryingChance()) { - // this server hasn't been isolated for enough long time, or there is no trying chance - return false; - } - // [1]we can implement better recovery based on several attempts, but here we do not know if this attempt is success - invocation.addLocalContext(TRYING_INSTANCES_EXISTING, Boolean.TRUE); - return true; + return ServiceCombServerStats.applyForTryingChance(invocation); } if (!serverStats.isIsolated()) { serverStats.markIsolated(true); diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java index 01dd6bc..4be8ff3 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java @@ -20,6 +20,8 @@ package org.apache.servicecomb.loadbalance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils; +import org.junit.After; import org.junit.Test; import mockit.Mock; @@ -29,6 +31,10 @@ import mockit.MockUp; * */ public class TestConfiguration { + @After + public void after() { + ArchaiusUtils.resetConfig(); + } @Test public void testConstants() { @@ -39,6 +45,7 @@ public class TestConfiguration { assertEquals("retryOnNext", Configuration.PROP_RETRY_ONNEXT); assertEquals("retryOnSame", Configuration.PROP_RETRY_ONSAME); assertEquals("SessionStickinessRule.successiveFailedTimes", Configuration.SUCCESSIVE_FAILED_TIMES); + assertEquals("maxSingleTestWindow", Configuration.FILTER_MAX_SINGLE_TEST_WINDOW); assertNotNull(Configuration.INSTANCE); } @@ -150,4 +157,12 @@ public class TestConfiguration { public void testGetSessionTimeoutInSeconds() { assertNotNull(Configuration.INSTANCE.getSessionTimeoutInSeconds("test")); } + + @Test + public void testGetMaxSingleTestWindow() { + assertEquals(60000, Configuration.INSTANCE.getMaxSingleTestWindow()); + + ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.maxSingleTestWindow", 5000); + assertEquals(5000, Configuration.INSTANCE.getMaxSingleTestWindow()); + } } diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java index b419921..6d71b47 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java @@ -79,7 +79,7 @@ public class TestLoadBalanceHandler2 { ArchaiusUtils.setProperty("servicecomb.loadbalance.userDefinedEndpoint.enabled", "true"); // avoid mock ServiceCombLoadBalancerStats.INSTANCE.init(); - ServiceCombServerStats.releaseTryingChance(); + TestServiceCombServerStats.releaseTryingChance(); mockTimeMillis = new Holder<>(1L); new MockUp<System>() { @@ -94,7 +94,7 @@ public class TestLoadBalanceHandler2 { public void teardown() { CseContext.getInstance().setTransportManager(null); ArchaiusUtils.resetConfig(); - ServiceCombServerStats.releaseTryingChance(); + TestServiceCombServerStats.releaseTryingChance(); } @Test @@ -854,7 +854,7 @@ public class TestLoadBalanceHandler2 { aysnc.success("OK"); }); - Assert.assertTrue(ServiceCombServerStats.applyForTryingChance()); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation)); invocation.addLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING, true); try { handler.handle(invocation, (response) -> Assert.assertEquals("OK", response.getResult())); @@ -911,7 +911,7 @@ public class TestLoadBalanceHandler2 { } }); - Assert.assertTrue(ServiceCombServerStats.applyForTryingChance()); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation)); invocation.addLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING, true); try { handler.handle(invocation, (response) -> Assert.assertEquals("OK", response.getResult())); diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombServerStats.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombServerStats.java index 3010368..ef1fd61 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombServerStats.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombServerStats.java @@ -17,16 +17,32 @@ package org.apache.servicecomb.loadbalance; +import java.util.Optional; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.common.Holder; +import org.apache.servicecomb.foundation.common.testing.MockClock; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import mockit.Mock; import mockit.MockUp; public class TestServiceCombServerStats { + @Before + public void before() { + releaseTryingChance(); + } + + @After + public void after() { + releaseTryingChance(); + } + @Test public void testSimpleThread() { long time = System.currentTimeMillis(); @@ -94,4 +110,47 @@ public class TestServiceCombServerStats { Assert.assertEquals(stats.getFailedRate(), 0); Assert.assertEquals(stats.getSuccessRate(), 100); } + + @Test + public void testGlobalAllowIsolatedServerTryingFlag_apply_with_null_precondition() { + Invocation invocation = new Invocation(); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation)); + Assert.assertSame(invocation, ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().getInvocation()); + } + + @Test + public void testGlobalAllowIsolatedServerTryingFlag_apply_with_chance_occupied() { + Invocation invocation = new Invocation(); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation)); + Assert.assertSame(invocation, ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().getInvocation()); + + Invocation otherInvocation = new Invocation(); + Assert.assertFalse(ServiceCombServerStats.applyForTryingChance(otherInvocation)); + Assert.assertSame(invocation, ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().getInvocation()); + } + + @Test + public void testGlobalAllowIsolatedServerTryingFlag_apply_with_flag_outdated() { + Invocation invocation = new Invocation(); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation)); + Assert.assertSame(invocation, ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().getInvocation()); + ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().clock = new MockClock(new Holder<>( + ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().startTryingTimestamp + 60000 + )); + + Invocation otherInvocation = new Invocation(); + Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(otherInvocation)); + Assert + .assertSame(otherInvocation, ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get().getInvocation()); + } + + public static void releaseTryingChance() { + ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.set(null); + } + + public static Invocation getTryingIsolatedServerInvocation() { + return Optional.ofNullable(ServiceCombServerStats.globalAllowIsolatedServerTryingFlag.get()) + .map(TryingIsolatedServerMarker::getInvocation) + .orElse(null); + } } diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java index a632352..d4c0acd 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java @@ -27,6 +27,7 @@ import org.apache.servicecomb.loadbalance.Configuration; import org.apache.servicecomb.loadbalance.ServiceCombLoadBalancerStats; import org.apache.servicecomb.loadbalance.ServiceCombServer; import org.apache.servicecomb.loadbalance.ServiceCombServerStats; +import org.apache.servicecomb.loadbalance.TestServiceCombServerStats; import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance; import org.apache.servicecomb.serviceregistry.cache.CacheEndpoint; import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext; @@ -80,13 +81,13 @@ public class IsolationDiscoveryFilterTest { discoveryTreeNode.data(data); filter = new IsolationDiscoveryFilter(); - ServiceCombServerStats.releaseTryingChance(); + TestServiceCombServerStats.releaseTryingChance(); } @After public void after() { Deencapsulation.invoke(ServiceCombLoadBalancerStats.INSTANCE, "init"); - ServiceCombServerStats.releaseTryingChance(); + TestServiceCombServerStats.releaseTryingChance(); } @Test @@ -137,8 +138,7 @@ public class IsolationDiscoveryFilterTest { ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server0, true); Assert.assertTrue(ServiceCombServerStats.isolatedServerCanTry()); - Assert.assertFalse( - Boolean.TRUE.equals(invocation.getLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING))); + Assert.assertNull(TestServiceCombServerStats.getTryingIsolatedServerInvocation()); DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode); Map<String, MicroserviceInstance> childNodeData = childNode.data(); Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2")); @@ -147,8 +147,7 @@ public class IsolationDiscoveryFilterTest { Assert.assertEquals(data.get("i2"), childNodeData.get("i2")); Assert.assertTrue(serviceCombServerStats.isIsolated()); Assert.assertFalse(ServiceCombServerStats.isolatedServerCanTry()); - Assert.assertTrue( - Boolean.TRUE.equals(invocation.getLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING))); + Assert.assertSame(invocation, TestServiceCombServerStats.getTryingIsolatedServerInvocation()); } @Test @@ -180,7 +179,8 @@ public class IsolationDiscoveryFilterTest { Assert.assertEquals(data.get("i1"), childNodeData.get("i1")); Assert.assertEquals(data.get("i2"), childNodeData.get("i2")); - ServiceCombServerStats.releaseTryingChance(); // after the first invocation releases the trying chance + ServiceCombServerStats + .checkAndReleaseTryingChance(invocation); // after the first invocation releases the trying chance // Other invocation can get the trying chance childNode = filter.discovery(discoveryContext, discoveryTreeNode);
