You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2019/12/19 06:08:05 UTC
[servicecomb-java-chassis] branch master updated: [SCB-1559] Use
java.time.Clock instead of java.lang.System#currentTimeMills to get time
(#1461)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new 7e7e887 [SCB-1559] Use java.time.Clock instead of java.lang.System#currentTimeMills to get time (#1461)
7e7e887 is described below
commit 7e7e88768af70ceffcdc10102d542d22c6274f33
Author: Ang Li <31...@users.noreply.github.com>
AuthorDate: Thu Dec 19 14:07:56 2019 +0800
[SCB-1559] Use java.time.Clock instead of java.lang.System#currentTimeMills to get time (#1461)
---
.../foundation/common/utils/TimeUtils.java | 35 ++++++++++++
.../test/scaffolding/model/MockClock.java | 52 +++++++++++++++++
.../qps/TestConsumerQpsFlowControlHandler.java | 7 ---
.../qps/TestProviderQpsFlowControlHandler.java | 14 -----
.../loadbalance/ServiceCombServerStats.java | 44 +++++++++++----
.../loadbalance/TestServiceCombServerStats.java | 65 +++++++++-------------
.../diagnosis/instance/InstanceCacheChecker.java | 6 +-
.../instance/TestInstanceCacheChecker.java | 9 +--
8 files changed, 154 insertions(+), 78 deletions(-)
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..476c390
--- /dev/null
+++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/TimeUtils.java
@@ -0,0 +1,35 @@
+/*
+ * 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/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/model/MockClock.java b/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/model/MockClock.java
new file mode 100644
index 0000000..88c1f23
--- /dev/null
+++ b/foundations/foundation-test-scaffolding/src/main/java/org/apache/servicecomb/foundation/test/scaffolding/model/MockClock.java
@@ -0,0 +1,52 @@
+/*
+ * 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.test.scaffolding.model;
+
+import javax.xml.ws.Holder;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+
+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/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
index 3e6bbba..b472a85 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
@@ -68,13 +68,6 @@ public class TestConsumerQpsFlowControlHandler {
@Test
public void testQpsController() {
- // to avoid time influence on QpsController
- new MockUp<System>() {
- @Mock
- long currentTimeMillis() {
- return 1L;
- }
- };
QpsController qpsController = new QpsController("abc", 100);
Assert.assertEquals(false, qpsController.isLimitNewRequest());
diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
index 9f73155..d5a7267 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
@@ -49,8 +49,6 @@ public class TestProviderQpsFlowControlHandler {
AsyncResponse asyncResp = Mockito.mock(AsyncResponse.class);
- OperationMeta operationMeta = Mockito.mock(OperationMeta.class);
-
@Rule
public ExpectedException expectedException = ExpectedException.none();
@@ -84,7 +82,6 @@ public class TestProviderQpsFlowControlHandler {
result = new RuntimeException("test error");
}
};
- mockUpSystemTime();
ProviderQpsFlowControlHandler gHandler = new ProviderQpsFlowControlHandler();
gHandler.handle(invocation, asyncResp);
@@ -100,7 +97,6 @@ public class TestProviderQpsFlowControlHandler {
@Test
public void testQpsController() {
- mockUpSystemTime();
QpsController qpsController = new QpsController("abc", 100);
assertFalse(qpsController.isLimitNewRequest());
@@ -181,14 +177,4 @@ public class TestProviderQpsFlowControlHandler {
Mockito.verify(invocation, times(0)).next(asyncResp);
Mockito.verify(asyncResp, times(0)).producerFail(Mockito.any(Exception.class));
}
-
- private void mockUpSystemTime() {
- // to avoid time influence on QpsController
- new MockUp<System>() {
- @Mock
- long currentTimeMillis() {
- return 1L;
- }
- };
- }
}
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..daf83dc 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,11 @@
package org.apache.servicecomb.loadbalance;
+import java.time.Clock;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
+import org.apache.servicecomb.foundation.common.utils.TimeUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -41,24 +43,46 @@ public class ServiceCombServerStats {
*/
private static AtomicBoolean globalAllowIsolatedServerTryingFlag = new AtomicBoolean(true);
- private long lastWindow = System.currentTimeMillis();
-
private final Object lock = new Object();
- private AtomicLong continuousFailureCount = new AtomicLong(0);
+ Clock clock;
+
+ private long lastWindow;
+
+ private AtomicLong continuousFailureCount;
- private long lastVisitTime = System.currentTimeMillis();
+ private long lastVisitTime;
- private long lastActiveTime = System.currentTimeMillis();
+ private long lastActiveTime;
- private AtomicLong totalRequests = new AtomicLong(0L);
+ private AtomicLong totalRequests;
- private AtomicLong successRequests = new AtomicLong(0L);
+ private AtomicLong successRequests;
- private AtomicLong failedRequests = new AtomicLong(0L);
+ private AtomicLong failedRequests;
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();
}
@@ -81,7 +105,7 @@ public class ServiceCombServerStats {
}
public void markSuccess() {
- long time = System.currentTimeMillis();
+ long time = clock.millis();
ensureWindow(time);
lastVisitTime = time;
lastActiveTime = time;
@@ -94,7 +118,7 @@ public class ServiceCombServerStats {
}
public void markFailure() {
- long time = System.currentTimeMillis();
+ long time = clock.millis();
ensureWindow(time);
lastVisitTime = time;
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..f2d8214 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
@@ -20,11 +20,11 @@ package org.apache.servicecomb.loadbalance;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import org.apache.servicecomb.foundation.test.scaffolding.model.MockClock;
import org.junit.Assert;
import org.junit.Test;
-import mockit.Mock;
-import mockit.MockUp;
+import javax.xml.ws.Holder;
public class TestServiceCombServerStats {
@Test
@@ -33,13 +33,13 @@ public class TestServiceCombServerStats {
ServiceCombServerStats stats = new ServiceCombServerStats();
stats.markFailure();
stats.markFailure();
- Assert.assertEquals(stats.getCountinuousFailureCount(), 2);
+ Assert.assertEquals(2, stats.getCountinuousFailureCount());
stats.markSuccess();
- Assert.assertEquals(stats.getCountinuousFailureCount(), 0);
+ Assert.assertEquals(0, stats.getCountinuousFailureCount());
stats.markSuccess();
- Assert.assertEquals(stats.getTotalRequests(), 4);
- Assert.assertEquals(stats.getFailedRate(), 50);
- Assert.assertEquals(stats.getSuccessRate(), 50);
+ Assert.assertEquals(4, stats.getTotalRequests());
+ Assert.assertEquals(50, stats.getFailedRate());
+ Assert.assertEquals(50, stats.getSuccessRate());
Assert.assertTrue(stats.getLastVisitTime() <= System.currentTimeMillis() && stats.getLastVisitTime() >= time);
Assert.assertTrue(stats.getLastActiveTime() <= System.currentTimeMillis() && stats.getLastActiveTime() >= time);
}
@@ -50,48 +50,35 @@ public class TestServiceCombServerStats {
ServiceCombServerStats stats = new ServiceCombServerStats();
CountDownLatch latch = new CountDownLatch(10);
for (int i = 0; i < 10; i++) {
- new Thread() {
- public void run() {
- stats.markFailure();
- stats.markFailure();
- stats.markSuccess();
- stats.markSuccess();
- latch.countDown();
- }
- }.start();
+ new Thread(() -> {
+ stats.markFailure();
+ stats.markFailure();
+ stats.markSuccess();
+ stats.markSuccess();
+ latch.countDown();
+ }).start();
}
latch.await(30, TimeUnit.SECONDS);
- Assert.assertEquals(stats.getTotalRequests(), 4 * 10);
- Assert.assertEquals(stats.getFailedRate(), 50);
- Assert.assertEquals(stats.getSuccessRate(), 50);
+ Assert.assertEquals(4 * 10, stats.getTotalRequests());
+ Assert.assertEquals(50, stats.getFailedRate());
+ Assert.assertEquals(50, stats.getSuccessRate());
Assert.assertTrue(stats.getLastVisitTime() <= System.currentTimeMillis() && stats.getLastVisitTime() >= time);
Assert.assertTrue(stats.getLastActiveTime() <= System.currentTimeMillis() && stats.getLastActiveTime() >= time);
}
@Test
public void testTimeWindow() {
- new MockUp<System>() {
- @Mock
- long currentTimeMillis() {
- return 1000;
- }
- };
- ServiceCombServerStats stats = new ServiceCombServerStats();
- Assert.assertEquals(stats.getLastVisitTime(), 1000);
+ ServiceCombServerStats stats = new ServiceCombServerStats(new MockClock(new Holder<>(1000L)));
+ Assert.assertEquals(1000, stats.getLastVisitTime());
stats.markSuccess();
stats.markFailure();
- Assert.assertEquals(stats.getTotalRequests(), 2);
- Assert.assertEquals(stats.getFailedRate(), 50);
- Assert.assertEquals(stats.getSuccessRate(), 50);
- new MockUp<System>() {
- @Mock
- long currentTimeMillis() {
- return 60000 + 2000;
- }
- };
+ Assert.assertEquals(2, stats.getTotalRequests());
+ Assert.assertEquals(50, stats.getFailedRate());
+ Assert.assertEquals(50, stats.getSuccessRate());
+ stats.clock = new MockClock(new Holder<>(60000L + 2000L));
stats.markSuccess();
- Assert.assertEquals(stats.getTotalRequests(), 1);
- Assert.assertEquals(stats.getFailedRate(), 0);
- Assert.assertEquals(stats.getSuccessRate(), 100);
+ Assert.assertEquals(1, stats.getTotalRequests());
+ Assert.assertEquals(0, stats.getFailedRate());
+ Assert.assertEquals(100, stats.getSuccessRate());
}
}
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java
index f086cc3..610a106 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java
@@ -16,12 +16,14 @@
*/
package org.apache.servicecomb.serviceregistry.diagnosis.instance;
+import java.time.Clock;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
+import org.apache.servicecomb.foundation.common.utils.TimeUtils;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
import org.apache.servicecomb.serviceregistry.client.http.MicroserviceInstances;
@@ -38,6 +40,8 @@ import io.vertx.core.json.Json;
public class InstanceCacheChecker {
private static final Logger LOGGER = LoggerFactory.getLogger(InstanceCacheChecker.class);
+
+ Clock clock = TimeUtils.getSystemDefaultZoneClock();
private AppManager appManager;
@@ -52,7 +56,7 @@ public class InstanceCacheChecker {
public InstanceCacheSummary check() {
instanceCacheSummary.setAppId(RegistryUtils.getMicroservice().getAppId());
instanceCacheSummary.setMicroserviceName(RegistryUtils.getMicroservice().getServiceName());
- instanceCacheSummary.setTimestamp(System.currentTimeMillis());
+ instanceCacheSummary.setTimestamp(clock.millis());
for (MicroserviceManager microserviceManager : appManager.getApps().values()) {
for (MicroserviceVersions microserviceVersions : microserviceManager.getVersionsByName().values()) {
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheChecker.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheChecker.java
index 3182a2b..e625768 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheChecker.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheChecker.java
@@ -21,6 +21,7 @@ import java.util.Arrays;
import javax.xml.ws.Holder;
+import org.apache.servicecomb.foundation.test.scaffolding.model.MockClock;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.ServiceRegistry;
import org.apache.servicecomb.serviceregistry.api.registry.Microservice;
@@ -58,15 +59,9 @@ public class TestInstanceCacheChecker {
RegistryUtils.setServiceRegistry(serviceRegistry);
checker = new InstanceCacheChecker(serviceRegistry.getAppManager());
+ checker.clock = new MockClock(new Holder<>(1L));
expectedSummary.setStatus(Status.NORMAL);
expectedSummary.setTimestamp(1);
-
- new MockUp<System>() {
- @Mock
- long currentTimeMillis() {
- return 1L;
- }
- };
}
@After