Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2203#discussion_r158555456
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -418,12 +430,24 @@ public DisruptorQueue(String queueName, ProducerType
type, int size, long readTi
_barrier = _buffer.newBarrier();
_buffer.addGatingSequences(_consumer);
_metrics = new QueueMetrics();
+ _disruptorMetrics =
StormMetricRegistry.disruptorMetrics(_queueName, topologyId, componentId, port);
//The batch size can be no larger than half the full queue size.
//This is mostly to avoid contention issues.
_inputBatchSize = Math.max(1, Math.min(inputBatchSize, size/2));
_flusher = new Flusher(Math.max(flushInterval, 1), _queueName);
_flusher.start();
+ try {
+ METRICS_TIMER.schedule(new TimerTask() {
+ @Override
+ public void run() {
+ _disruptorMetrics.set(_metrics);
+ }
+ }, 15000, 15000);
+ } catch (IllegalStateException e){
--- End diff --
Thanks, but this seems like it could hide errors by accident. If we replace
the Timer with a
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ScheduledThreadPoolExecutor.html
we can check if it is shut down instead of catching this exception, and do a
debug or trace log if it is, so it's obvious why scheduling isn't happening in
case someone has to debug this later.
I don't know why it isn't being shown in the current diff, but there's also
this suggestion from earlier that doesn't appear to have a response:
https://github.com/apache/storm/pull/2203/files/00a382b017c1e29863ac4d9a4449086ef79384e4#r128586571
---
