-
Notifications
You must be signed in to change notification settings - Fork 640
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
[Issue #337] Enhance Http Demo Subscriber by using ExecutorService, CountDownLatch and PreDestroy hook #343
Changes from 7 commits
ba67c5c
d638ec4
5ebfb54
a3afff3
50f959d
7adc322
d48ead5
c9021fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,19 +3,25 @@ | |
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Properties; | ||
import java.util.concurrent.CountDownLatch; | ||
import java.util.concurrent.ExecutorService; | ||
import java.util.concurrent.Executors; | ||
|
||
import org.apache.commons.lang3.StringUtils; | ||
import org.apache.eventmesh.client.http.conf.LiteClientConfig; | ||
import org.apache.eventmesh.client.http.consumer.LiteConsumer; | ||
import org.apache.eventmesh.common.EventMeshException; | ||
import org.apache.eventmesh.common.IPUtil; | ||
import org.apache.eventmesh.common.ThreadUtil; | ||
import org.apache.eventmesh.http.demo.AsyncPublishInstance; | ||
import org.apache.eventmesh.util.Utils; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.InitializingBean; | ||
import org.springframework.stereotype.Component; | ||
|
||
import javax.annotation.PreDestroy; | ||
|
||
@Component | ||
public class SubService implements InitializingBean { | ||
|
||
|
@@ -38,6 +44,11 @@ public class SubService implements InitializingBean { | |
final String dcn = "FT0"; | ||
final String subsys = "1234"; | ||
|
||
// CountDownLatch size is the same as messageSize in AsyncPublishInstance.java (Publisher) | ||
private CountDownLatch countDownLatch = new CountDownLatch(AsyncPublishInstance.messageSize); | ||
|
||
private ExecutorService executorService = Executors.newFixedThreadPool(5); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of cource it won't make a difference, but I wonder here is it necessary to use a thread pool? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a best practice to use the JDK Concurrency library (ExecutorService + ThreadPool) to manage the lifecycle of Thread. Just suggestion. I can rollback to create Thread manually. Thanks for review comment. |
||
|
||
@Override | ||
public void afterPropertiesSet() throws Exception { | ||
|
||
|
@@ -59,31 +70,41 @@ public void afterPropertiesSet() throws Exception { | |
liteConsumer.heartBeat(topicList, url); | ||
liteConsumer.subscribe(topicList, url); | ||
|
||
Runtime.getRuntime().addShutdownHook(new Thread(() -> { | ||
logger.info("start destory ...."); | ||
try { | ||
liteConsumer.unsubscribe(topicList, url); | ||
} catch (EventMeshException e) { | ||
e.printStackTrace(); | ||
} | ||
// Wait for all messaged to be consumed | ||
executorService.submit(() ->{ | ||
try { | ||
liteConsumer.shutdown(); | ||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
} | ||
logger.info("end destory."); | ||
})); | ||
|
||
Thread stopThread = new Thread(() -> { | ||
try { | ||
Thread.sleep(5 * 60 * 1000); | ||
countDownLatch.await(); | ||
} catch (InterruptedException e) { | ||
e.printStackTrace(); | ||
} | ||
logger.info("stopThread start...."); | ||
System.exit(0); | ||
}); | ||
} | ||
|
||
@PreDestroy | ||
public void cleanup() { | ||
logger.info("start destory ...."); | ||
try { | ||
liteConsumer.unsubscribe(topicList, url); | ||
} catch (EventMeshException e) { | ||
e.printStackTrace(); | ||
} | ||
try { | ||
liteConsumer.shutdown(); | ||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
} | ||
executorService.shutdown(); | ||
logger.info("end destory."); | ||
} | ||
|
||
stopThread.start(); | ||
/** | ||
* Count the message already consumed | ||
*/ | ||
public void consumeMessage(String msg) { | ||
logger.info("consume message {}", msg); | ||
countDownLatch.countDown(); | ||
logger.info("remaining number of messages to be consumed {}", countDownLatch.getCount()); | ||
} | ||
} |
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.
The semantics of the 47 code line looks like it should pass in the received message as a parameter, not the result of consumption
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.
Good comment! I will make the change to pass the message instead of result.