[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187330052
##
File path:
handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationServerListFilter.java
##
@@ -91,19 +94,20 @@ private boolean allowVisit(Server server) {
ServerStats serverStats = stats.getSingleServerStat(server);
long totalRequest = serverStats.getTotalRequestsCount();
long failureRequest = serverStats.getSuccessiveConnectionFailureCount();
-
+int currentCountinuousFailureCount = ((CseServer)
server).getCountinuousFailureCount();
Review comment:
why move calc to this place?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187329477
##
File path:
handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java
##
@@ -0,0 +1,99 @@
+/*
+ * 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.event;
+
+import java.util.HashMap;
+
+import org.apache.servicecomb.foundation.common.event.AlarmEvent;
+
+public class IsolationServerEvent extends AlarmEvent {
+
+ private String microserviceName;
+
+ //当前实例总请求数
+ private long currentTotalRequest;
+
+ //当前实例连续出错次数
+ private int currentCountinuousFailureCount;
+
+ //当前实例出错百分比
+ private double currentErrorPercentage;
+
+ private long enableRequestThreshold;
+
+ private int continuousFailureThreshold;
+
+ private int errorThresholdPercentage;
+
+ private long singleTestTime;
+
+ public IsolationServerEvent(String microserviceName, long totalRequest, int
currentCountinuousFailureCount,
+ double currentErrorPercentage, int continuousFailureThreshold,
+ int errorThresholdPercentage, long enableRequestThreshold, long
singleTestTime, Type type) {
+super(type);
+HashMap msg = new HashMap<>();
+this.microserviceName = microserviceName;
+this.currentTotalRequest = totalRequest;
+this.currentCountinuousFailureCount = currentCountinuousFailureCount;
+this.currentErrorPercentage = currentErrorPercentage;
+this.enableRequestThreshold = enableRequestThreshold;
+this.continuousFailureThreshold = continuousFailureThreshold;
+this.errorThresholdPercentage = errorThresholdPercentage;
+this.singleTestTime = singleTestTime;
+msg.put("microserviceName", this.microserviceName);
Review comment:
why we need not only model, but also map?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187329270
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/event/CircutBreakerEventNotifier.java
##
@@ -0,0 +1,58 @@
+/*
+ * 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.bizkeeper.event;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.servicecomb.foundation.common.event.EventManager;
+import org.apache.servicecomb.foundation.common.event.AlarmEvent.Type;
+
+import com.netflix.hystrix.HystrixCommandKey;
+import com.netflix.hystrix.HystrixEventType;
+import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
+
+public class CircutBreakerEventNotifier extends HystrixEventNotifier {
+
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ */
+ private static ConcurrentHashMap circuitMarker = new
ConcurrentHashMap<>();
+
+ @Override
+ public void markEvent(HystrixEventType eventType, HystrixCommandKey key) {
+String keyName = key.name();
+switch (eventType) {
+ case SHORT_CIRCUITED:
+if (circuitMarker.get(keyName) == null) {
Review comment:
ConcurrentHashMapEx circuitFlag = new
ConcurrentHashMapEx<>();
AtomicBoolean flag = circuitFlag.computeIfAbsent(key, k -> new
AtomicBoolean());
// circuit event
if (flag.compareAndSet(false, true)) {
// post xxx event
}
// success event
if (flag.compareAndSet(true, false)) {
// post xxx event
}
---
and infact, i don't know what does eventType mean,
open/close/succeeded/failed
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187239668
##
File path:
handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/TestIsolationServerListFilter.java
##
@@ -60,29 +63,26 @@ public static void classTeardown() {
Deencapsulation.setField(ConfigurationManager.class, "instance", null);
Deencapsulation.setField(ConfigurationManager.class,
"customConfigurationInstalled", false);
Deencapsulation.setField(DynamicPropertyFactory.class, "config", null);
+ArchaiusUtils.resetConfig();
}
@Before
public void setUp() throws Exception {
IsolationServerListFilter = new IsolationServerListFilter();
loadBalancerStats = new LoadBalancerStats("loadBalancer");
-
-AbstractConfiguration configuration =
-(AbstractConfiguration)
DynamicPropertyFactory.getBackingConfigurationSource();
-configuration.clearProperty("cse.loadbalance.isolation.enabled");
-configuration.addProperty("cse.loadbalance.isolation.enabled",
+ArchaiusUtils.setProperty("cse.loadbalance.isolation.enabled",
"true");
-
configuration.clearProperty("cse.loadbalance.isolation.enableRequestThreshold");
-
configuration.addProperty("cse.loadbalance.isolation.enableRequestThreshold",
+
ArchaiusUtils.setProperty("cse.loadbalance.isolation.enableRequestThreshold",
"3");
taskList = new ArrayList<>();
-EventManager.register(new Object() {
+receiveEvent = new Object() {
@Subscribe
public void onEvent(AlarmEvent isolationServerEvent) {
taskList.add(isolationServerEvent);
}
-});
+};
+EventManager.register(receiveEvent);
Review comment:
EventManager is global instance
where is the clear action?
and it's better not do this
also had wrote this days ago
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187239487
##
File path:
handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/TestIsolationServerListFilter.java
##
@@ -60,29 +63,26 @@ public static void classTeardown() {
Deencapsulation.setField(ConfigurationManager.class, "instance", null);
Deencapsulation.setField(ConfigurationManager.class,
"customConfigurationInstalled", false);
Deencapsulation.setField(DynamicPropertyFactory.class, "config", null);
+ArchaiusUtils.resetConfig();
Review comment:
already ArchaiusUtils.resetConfig()
why need other reflections?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187238597
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/event/CircutBreakerEventNotifier.java
##
@@ -0,0 +1,58 @@
+/*
+ * 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.bizkeeper.event;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.servicecomb.foundation.common.event.EventManager;
+import org.apache.servicecomb.foundation.common.event.AlarmEvent.Type;
+
+import com.netflix.hystrix.HystrixCommandKey;
+import com.netflix.hystrix.HystrixEventType;
+import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
+
+public class CircutBreakerEventNotifier extends HystrixEventNotifier {
+
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ */
+ private static ConcurrentHashMap circuitMarker = new
ConcurrentHashMap<>();
+
+ @Override
+ public void markEvent(HystrixEventType eventType, HystrixCommandKey key) {
+String keyName = key.name();
+switch (eventType) {
+ case SHORT_CIRCUITED:
+if (circuitMarker.get(keyName) == null) {
Review comment:
remember you said that, subscribers not only need switch event, but also
every invocation that circuited.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r187238232
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/event/CircutBreakerEventNotifier.java
##
@@ -0,0 +1,58 @@
+/*
+ * 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.bizkeeper.event;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.servicecomb.foundation.common.event.EventManager;
+import org.apache.servicecomb.foundation.common.event.AlarmEvent.Type;
+
+import com.netflix.hystrix.HystrixCommandKey;
+import com.netflix.hystrix.HystrixEventType;
+import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
+
+public class CircutBreakerEventNotifier extends HystrixEventNotifier {
+
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ */
+ private static ConcurrentHashMap circuitMarker = new
ConcurrentHashMap<>();
+
+ @Override
+ public void markEvent(HystrixEventType eventType, HystrixCommandKey key) {
+String keyName = key.name();
+switch (eventType) {
+ case SHORT_CIRCUITED:
+if (circuitMarker.get(keyName) == null) {
Review comment:
concurrency problem
i had wrote this problem days ago.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报 URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186251122 ## File path: handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/TestIsolationServerListFilter.java ## @@ -45,6 +48,8 @@ LoadBalancerStats loadBalancerStats = null; + private List taskList; + Review comment: teardown: org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils.resetConfig This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186251088
##
File path:
handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/TestIsolationServerListFilter.java
##
@@ -70,6 +75,14 @@ public void setUp() throws Exception {
configuration.clearProperty("cse.loadbalance.isolation.enableRequestThreshold");
Review comment:
org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils.setProperty
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186251045
##
File path:
handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/TestIsolationServerListFilter.java
##
@@ -70,6 +75,14 @@ public void setUp() throws Exception {
configuration.clearProperty("cse.loadbalance.isolation.enableRequestThreshold");
configuration.addProperty("cse.loadbalance.isolation.enableRequestThreshold",
"3");
+
+taskList = new ArrayList<>();
+EventManager.register(new Object() {
Review comment:
global instance
it's better do not use this directly.
you can save a reference in filter
when UT create a new instance for it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186251016
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -74,6 +83,7 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
HystrixObservable command =
delegate.createBizkeeperCommand(invocation);
Observable observable = command.toObservable();
+eventAlarm(invocation);
Review comment:
// record that we are returning a short-circuited fallback
eventNotifier.markEvent(HystrixEventType.SHORT_CIRCUITED,
commandKey);
when allowRequest return false, will invoke this, it seems that we no need
to check everytime.
please check is it possible to optimize
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186250938
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -82,6 +92,25 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
});
}
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ * 判断熔断开启或关闭，如果之前没有发送过告警信息才会发送告警
+ */
+ private void eventAlarm(Invocation invocation) {
+HystrixCommandKey commandKey = CommandKey.toHystrixCommandKey(groupname,
invocation);
+HystrixCircuitBreaker circuitBreaker =
+HystrixCircuitBreaker.Factory.getInstance(commandKey);
+String microserviceQualifiedName =
invocation.getMicroserviceQualifiedName();
+if (circuitBreaker != null && circuitBreaker.isOpen() &&
circuitMarker.get(microserviceQualifiedName) == null) {
+ EventManager.post(new CircutBreakerEvent(invocation, commandKey,
this.groupname, Type.OPEN));
Review comment:
both?
by your code, only when break flag changed will post event.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r186250923
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/event/CircutBreakerEvent.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.bizkeeper.event;
+
+import java.util.HashMap;
+
+import org.apache.servicecomb.bizkeeper.Configuration;
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.common.event.AlarmEvent;
+
+import com.netflix.hystrix.HystrixCommandKey;
+import com.netflix.hystrix.HystrixCommandMetrics;
+
+public class CircutBreakerEvent extends AlarmEvent {
+
+ private static int id = 1001;
+
+ private HashMap msg = new HashMap<>();
Review comment:
someone subscribe CircutBreakerEvent, how to know what's information in it?
1.read doc?
2.run and watch the instance?
why not define model, so that developers can get everything by class?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185535372
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -82,6 +92,25 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
});
}
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ * 判断熔断开启或关闭，如果之前没有发送过告警信息才会发送告警
+ */
+ private void eventAlarm(Invocation invocation) {
+HystrixCommandKey commandKey = CommandKey.toHystrixCommandKey(groupname,
invocation);
+HystrixCircuitBreaker circuitBreaker =
+HystrixCircuitBreaker.Factory.getInstance(commandKey);
+String microserviceQualifiedName =
invocation.getMicroserviceQualifiedName();
+if (circuitBreaker != null && circuitBreaker.isOpen() &&
circuitMarker.get(microserviceQualifiedName) == null) {
+ EventManager.post(new CircutBreakerEvent(invocation, commandKey,
this.groupname, Type.OPEN));
Review comment:
requirement is want to know the even that some one invocation is broken
or break flag change from one to another
or both?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185535372
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -82,6 +92,25 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
});
}
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ * 判断熔断开启或关闭，如果之前没有发送过告警信息才会发送告警
+ */
+ private void eventAlarm(Invocation invocation) {
+HystrixCommandKey commandKey = CommandKey.toHystrixCommandKey(groupname,
invocation);
+HystrixCircuitBreaker circuitBreaker =
+HystrixCircuitBreaker.Factory.getInstance(commandKey);
+String microserviceQualifiedName =
invocation.getMicroserviceQualifiedName();
+if (circuitBreaker != null && circuitBreaker.isOpen() &&
circuitMarker.get(microserviceQualifiedName) == null) {
+ EventManager.post(new CircutBreakerEvent(invocation, commandKey,
this.groupname, Type.OPEN));
Review comment:
requirement is want to know the event that some one invocation is broken
or break flag change from one to another
or both?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185534113
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/event/CircutBreakerEvent.java
##
@@ -0,0 +1,67 @@
+/*
+ * 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.bizkeeper.event;
+
+import java.util.HashMap;
+
+import org.apache.servicecomb.bizkeeper.Configuration;
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.foundation.common.event.AlarmEvent;
+
+import com.netflix.hystrix.HystrixCommandKey;
+import com.netflix.hystrix.HystrixCommandMetrics;
+
+public class CircutBreakerEvent extends AlarmEvent {
+
+ private static int id = 1001;
+
+ private HashMap msg = new HashMap<>();
Review comment:
every key is known, why not define a model?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185533766
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -74,6 +83,7 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
HystrixObservable command =
delegate.createBizkeeperCommand(invocation);
Observable observable = command.toObservable();
+eventAlarm(invocation);
Review comment:
need this logic every time ?
we can not know when will do the break action?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185533065
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -82,6 +92,25 @@ public void handle(Invocation invocation, AsyncResponse
asyncResp) {
});
}
+ /**
+ * 使用circuitMarker来记录被熔断的接口
+ * 判断熔断开启或关闭，如果之前没有发送过告警信息才会发送告警
+ */
+ private void eventAlarm(Invocation invocation) {
+HystrixCommandKey commandKey = CommandKey.toHystrixCommandKey(groupname,
invocation);
+HystrixCircuitBreaker circuitBreaker =
+HystrixCircuitBreaker.Factory.getInstance(commandKey);
+String microserviceQualifiedName =
invocation.getMicroserviceQualifiedName();
+if (circuitBreaker != null && circuitBreaker.isOpen() &&
circuitMarker.get(microserviceQualifiedName) == null) {
+ EventManager.post(new CircutBreakerEvent(invocation, commandKey,
this.groupname, Type.OPEN));
+ circuitMarker.put(microserviceQualifiedName, true);
+} else if (circuitBreaker != null && !circuitBreaker.isOpen()
+&& circuitMarker.get(microserviceQualifiedName) != null) {
+ EventManager.post(new CircutBreakerEvent(invocation, commandKey,
this.groupname, Type.CLOSE));
Review comment:
even circuitMarker no concurrency problem
this logic is not good.
multi threads run if conditions and got true, then all run into post and
remove logic.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185532292
##
File path:
handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/BizkeeperHandler.java
##
@@ -39,6 +46,8 @@
public abstract class BizkeeperHandler implements Handler {
private static final Logger LOG =
LoggerFactory.getLogger(BizkeeperHandler.class);
+ private static HashMap circuitMarker = new HashMap<>();
Review comment:
concurrency problem?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
wujimin commented on a change in pull request #678: [SCB-506]服务治理相关的需要事件上报
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/678#discussion_r185531931
##
File path:
foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/AlarmEvent.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.foundation.common.event;
+
+import java.util.HashMap;
+
+public class AlarmEvent {
+
+ Type type;
+
+ private int id;
+
+ private HashMap msg;
+
+ public AlarmEvent(Type type, int id) {
+this.type = type;
+this.id = id;
+ }
+
+ public Type getType() {
+return this.type;
+ }
+
+ public int getId() {
+return id;
+ }
+
+ public HashMap getMsg() {
+return msg;
+ }
+
+ public void setMsg(HashMap msg2) {
+this.msg = msg2;
+ }
+
+ public enum Type {
+OPEN,
+CLOSE,
+SUCCEDED,
Review comment:
SUCCEEDED?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
