dschneider-pivotal commented on a change in pull request #5763:
URL: https://github.com/apache/geode/pull/5763#discussion_r527838255
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/monitoring/ThreadsMonitoringImpl.java
##########
@@ -96,39 +98,60 @@ public void updateThreadStatus() {
@Override
public boolean startMonitor(Mode mode) {
- AbstractExecutor absExtgroup;
+ return startMonitoring(createAbstractExecutor(mode));
+ }
+
+ @Override
+ public void endMonitor() {
+ this.monitorMap.remove(Thread.currentThread().getId());
+ }
+
+ @VisibleForTesting
+ boolean isMonitoring() {
+ return monitorMap.containsKey(Thread.currentThread().getId());
+ }
+
+ @Override
+ public AbstractExecutor createAbstractExecutor(Mode mode) {
switch (mode) {
case FunctionExecutor:
- absExtgroup = new FunctionExecutionPooledExecutorGroup(this);
- break;
+ return new FunctionExecutionPooledExecutorGroup();
case PooledExecutor:
- absExtgroup = new PooledExecutorGroup(this);
- break;
+ return new PooledExecutorGroup();
case SerialQueuedExecutor:
- absExtgroup = new SerialQueuedExecutorGroup(this);
- break;
+ return new SerialQueuedExecutorGroup();
case OneTaskOnlyExecutor:
- absExtgroup = new OneTaskOnlyExecutorGroup(this);
- break;
+ return new OneTaskOnlyExecutorGroup();
case ScheduledThreadExecutor:
- absExtgroup = new ScheduledThreadPoolExecutorWKAGroup(this);
- break;
+ return new ScheduledThreadPoolExecutorWKAGroup();
case AGSExecutor:
- absExtgroup = new GatewaySenderEventProcessorGroup(this);
- break;
+ return new GatewaySenderEventProcessorGroup();
+ case P2PReaderExecutor:
+ return new P2PReaderExecutorGroup();
default:
- return false;
+ throw new IllegalStateException("Unhandled mode=" + mode);
}
- this.monitorMap.put(Thread.currentThread().getId(), absExtgroup);
+ }
+
+ @Override
+ public boolean startMonitoring(AbstractExecutor executor) {
+ executor.setStartTime(System.currentTimeMillis());
Review comment:
I had the same thought. I was even concerned about constantly adding and
removing from the concurrent map. We could just use the frequency of the
monitor thread to do the time work. For example if it finds an AbstractExecutor
in the map that it has not seen before then the monitor thread could set the
time it first saw it. If later monitor samples see it again it could use this
time (i.e. the one the monitor set) to determine if it is stuck. This could
even be done without timestamps. Just a simple counter the monitor incs and the
p2p reader thread clears. If the monitor thread is waking up at a fixed
interval then you know the thread has been stuck for at least that much time. I
like your idea of having custom code in P2PReaderExecutorGroup for this. When I
was thinking about it I thought I would need to change the behavior of all the
existing threads being monitored. Thanks for the feedback!
----------------------------------------------------------------
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]
