nwangtw commented on a change in pull request #3355: Add a cap to metrics
communicator in TMasterSink and MetricsCacheSink
URL: https://github.com/apache/incubator-heron/pull/3355#discussion_r331605660
##########
File path:
heron/metricsmgr/src/java/org/apache/heron/metricsmgr/sink/metricscache/MetricsCacheClient.java
##########
@@ -93,12 +93,20 @@ private void addMetricsCacheClientTasksOnWakeUp() {
Runnable task = new Runnable() {
@Override
public void run() {
- while (!publishMetricsCommunicator.isEmpty()) {
- TopologyMaster.PublishMetrics publishMetrics =
publishMetricsCommunicator.poll();
+ TopologyMaster.PublishMetrics publishMetrics;
+ synchronized (publishMetricsCommunicator) {
Review comment:
Yeah. Before there were only 1 producer (metrics manager) and 1 consumer
(sink client) for this communicator from two different threads.
In the change, the producer can remove entries from head too so it is
competing with the consumer thread. A simpler way is to avoid adding new
entries into the queue, then we don't need to synchronize the producer and the
consumer. However, I feel that the recent data could be more important than the
old data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
