-
Notifications
You must be signed in to change notification settings - Fork 837
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#513] Virtual thread pin event metric and alarm #517
base: springboot3-dev
Are you sure you want to change the base?
[#513] Virtual thread pin event metric and alarm #517
Conversation
[ISSUE dromara#515] add scheduledFuture cancel check
…nto springboot3
代码改的太乱,改动太大了。我理解这是一个新功能,期望的应该是以最小代价、最小改动集成进去,对扩展开放,对修改关闭,而不是大量改动现有逻辑,分散在其中。 |
public static final String LARK_ALARM_JSON_COMMON_STR = | ||
"{\"tag\":\"div\",\"fields\":[{\"is_short\":true,\"text\":{\"tag\":\"lark_md\",\"content\":\"%s\"}}]},"; | ||
|
||
public static final String LARK_ALARM_JSON_STR_SUFFIX = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是一个完整的json,不要拆分开,很难维护,格式容易配置错
/** | ||
* Pin timeout alarm. | ||
*/ | ||
PIN_TIMEOUT("pin_timeout", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个不要归类到该枚举里,枚举里是线程池层面常规项,这个加入不太合适
/** | ||
* 拓展字段 | ||
*/ | ||
private final Map<String, Double> extMap = new ConcurrentHashMap<>(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
扩展字段类型可以是任意,不要用double,用Object,初始长度不用指定
@@ -102,11 +102,16 @@ public static List<NotifyItem> getAllNotifyItems() { | |||
queueTimeoutNotify.setType(NotifyItemEnum.QUEUE_TIMEOUT.getValue()); | |||
queueTimeoutNotify.setThreshold(10); | |||
|
|||
List<NotifyItem> notifyItems = new ArrayList<>(6); | |||
NotifyItem pinTimeoutNotify = new NotifyItem(); | |||
pinTimeoutNotify.setType(NotifyItemEnum.PIN_TIMEOUT.getValue()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个不要归类到现有类型里
@@ -220,6 +221,10 @@ private static void refresh(ExecutorWrapper executorWrapper, DtpExecutorProps pr | |||
|
|||
private static void doRefresh(ExecutorWrapper executorWrapper, DtpExecutorProps props) { | |||
ExecutorAdapter<?> executor = executorWrapper.getExecutor(); | |||
if (executorWrapper.isVirtualThreadExecutor()) { | |||
doRefreshCommon(executorWrapper, props); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个判断可以去掉?走现有逻辑
NotifyItem notifyItem = DtpNotifyCtxHolder.get().getNotifyItem(); | ||
BaseNotifyCtx notifyCtx = DtpNotifyCtxHolder.get(); | ||
if (notifyCtx.isToLog()) { | ||
log.warn("DynamicTp alarm, executor [" + notifyCtx.getExecutorWrapper().getThreadPoolName() + "]: \n" + Arrays.toString(notifyCtx.getContent())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里是业务无关逻辑,不要硬加判断搞的很乱
if (notifyCtx.isCommonNotify()) { | ||
notifier.sendCommonAlarmMsg(p, notifyItemEnum, notifyCtx.getContent()); | ||
} else { | ||
if (!notifyCtx.getExecutorWrapper().isVirtualThreadExecutor()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
@@ -35,11 +35,24 @@ public class BaseNotifyCtx { | |||
|
|||
private NotifyItem notifyItem; | |||
|
|||
private boolean isCommonNotify; | |||
|
|||
private boolean isToLog; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不懂加这些字段意义是啥
@@ -88,12 +89,22 @@ public void sendAlarmMsg(NotifyPlatform notifyPlatform, NotifyItemEnum notifyIte | |||
notifier.send(newTargetPlatform(notifyPlatform), content); | |||
} | |||
|
|||
@Override | |||
public void sendCommonAlarmMsg(NotifyPlatform notifyPlatform, NotifyItemEnum notifyItemEnum, String... content) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
通用告警接口期望的是调用方传进来最终content,直接发送,不用再做组装
编译时需要将core,spring模块的JDK版本更改为21 |
[#513] Virtual thread pin event metric and alarm