You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/09/11 07:47:52 UTC
[GitHub] [inlong] liangyepianzhou opened a new pull request, #5852: [INLONG-5851][TubeMq]Optimize while-sleep to ScheduledExecutorService in tubeMQ
liangyepianzhou opened a new pull request, #5852:
URL: https://github.com/apache/inlong/pull/5852
### Prepare a Pull Request
- Fixes https://github.com/apache/inlong/issues/5851
### Motivation
it is not recommended to execute a scheduled task by while-sleep.
### Modifications
Replace while-sleep with ScheduledExecutorService.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gosonzhang commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r969075692
##########
inlong-tubemq/tubemq-manager/pom.xml:
##########
@@ -176,6 +176,10 @@
<groupId>jakarta.validation</groupId>
<artifactId>jakarta.validation-api</artifactId>
</dependency>
+ <dependency>
Review Comment:
Need to add the LICENSE and NOTICE (if any) that the third party depends on in [1]
https://github.com/apache/inlong/blob/master/licenses/inlong-tubemq-manager/
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang merged pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService in tubemq-manager
Posted by GitBox <gi...@apache.org>.
dockerzhang merged PR #5852:
URL: https://github.com/apache/inlong/pull/5852
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang commented on pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService
Posted by GitBox <gi...@apache.org>.
dockerzhang commented on PR #5852:
URL: https://github.com/apache/inlong/pull/5852#issuecomment-1247829521
please remove the LICENSE-netty.txt and NOTICE-netty.txt file.
for apache license dependency, you can add it to LICENSE file directly. you can refer to the [netty-common](https://github.com/apache/inlong/blob/master/licenses/inlong-manager/LICENSE#L505) in inlong-manager.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService in tubemq-manager
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r973703587
##########
inlong-tubemq/tubemq-manager/pom.xml:
##########
@@ -176,6 +176,10 @@
<groupId>jakarta.validation</groupId>
<artifactId>jakarta.validation-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-common</artifactId>
Review Comment:
The DefaultThreadFactory in TopicBackendWorker.java
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService
Posted by GitBox <gi...@apache.org>.
dockerzhang commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r971740319
##########
inlong-tubemq/tubemq-manager/pom.xml:
##########
@@ -176,6 +176,11 @@
<groupId>jakarta.validation</groupId>
<artifactId>jakarta.validation-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-common</artifactId>
+ <version>4.1.77.Final</version>
Review Comment:
version is not needed here.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService in tubemq-manager
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r977258424
##########
inlong-tubemq/tubemq-manager/src/main/java/org/apache/inlong/tubemq/manager/service/TopicBackendWorker.java:
##########
@@ -136,5 +140,6 @@ public void run() {
public void destroy() throws Exception {
runFlag.set(false);
nodeService.close();
+ this.workerExecutor.shutdown();
Review Comment:
Maybe use `shutdownNow()`?
##########
inlong-tubemq/tubemq-manager/src/main/java/org/apache/inlong/tubemq/manager/service/TopicBackendWorker.java:
##########
@@ -67,9 +71,10 @@ public class TopicBackendWorker implements DisposableBean, Runnable {
TopicBackendWorker() {
Thread thread = new Thread(this);
- // daemon thread
- thread.setDaemon(true);
- thread.start();
+ this.workerExecutor = Executors
+ .newSingleThreadScheduledExecutor(
Review Comment:
Introducing a package just to use a ThreadFactory is not a good solution.
It is recommended to use the `com.google.common.util.concurrent.ThreadFactoryBuilder` class from the guava package to create a ThreadFactory.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService in tubemq-manager
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r977385911
##########
licenses/inlong-tubemq-manager/NOTICE:
##########
@@ -494,6 +494,10 @@ Jakarta Annotations API NOTICE
Jakarta Persistence API NOTICE
------------------------------------------------------
javax.transaction API NOTICE
+------------------------------------------------------
+Netty NOTICE
Review Comment:
Please remove those notices.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #5852: [INLONG-5851][TubeMQ] Optimize while-sleep to ScheduledExecutorService in tube-manager
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #5852:
URL: https://github.com/apache/inlong/pull/5852#discussion_r973647501
##########
inlong-tubemq/tubemq-manager/pom.xml:
##########
@@ -176,6 +176,10 @@
<groupId>jakarta.validation</groupId>
<artifactId>jakarta.validation-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-common</artifactId>
Review Comment:
Excuse me, where did you use the netty-common package?
##########
licenses/inlong-tubemq-manager/LICENSE:
##########
@@ -452,6 +452,7 @@ The text of each license is also included at licenses/LICENSE-[project].txt.
org.springframework:spring-web:5.3.20 - Spring Web (https://github.com/spring-projects/spring-framework), (Apache License, Version 2.0)
org.springframework:spring-webmvc:5.3.18 - Spring Web MVC (https://github.com/spring-projects/spring-framework), (Apache License, Version 2.0)
org.apache.tomcat.embed:tomcat-embed-core:9.0.60 - tomcat-embed-core (https://tomcat.apache.org/), (Apache License, Version 2.0)
+ io.netty:netty-common:4.1.72.Final - Netty/Common (https://github.com/netty/netty/tree/netty-4.1.72.Final), (Apache License, Version 2.0)
Review Comment:
If the netty-common jar is needed, it should be sorted alphabetically by artifactId, here it's `netty-common`.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org