Aias00 commented on code in PR #7275:
URL: https://github.com/apache/shenyu/pull/7275#discussion_r4110028536
##########
shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-aliyun-sls/src/main/java/org/apache/shenyu/plugin/aliyun/sls/client/AliyunSlsLogCollectClient.java:
##########
@@ -191,7 +191,7 @@ private static ThreadPoolExecutor
createThreadPoolExecutor(final AliyunLogCollec
}
return new ThreadPoolExecutor(sendThreadCount,
GenericLoggingConstant.MAX_ALLOW_THREADS, 60000L, TimeUnit.MILLISECONDS,
new
LinkedBlockingQueue<>(GenericLoggingConstant.MAX_QUEUE_NUMBER),
ShenyuThreadFactory.create("shenyu-aliyun-sls", true),
- new ThreadPoolExecutor.AbortPolicy());
+ new ThreadPoolExecutor.CallerRunsPolicy());
Review Comment:
Non-blocking, but please check: under CallerRunsPolicy this callback now
runs inline on whichever thread completes the future submitted at
AliyunSlsLogCollectClient.java:144 (`Futures.addCallback(f, new
ProducerFutureCallback(projectName, logStore), threadExecutor)`) - normally the
SLS SDK sender thread, or the log collector consumer thread when the future is
already complete at registration time. Both are fine places to absorb
backpressure. Could you confirm the SLS SDK never completes it on a Netty
event-loop thread that also serves traffic? The same question applies to the
identical change in the Huawei LTS (HuaweiLtsLogCollectClient.java:180) and
Tencent CLS (TencentClsLogCollectClient.java:173) clients. Worth writing the
answer into the javadoc above `createThreadPoolExecutor` so the assumption
survives the next refactor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
