geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Udo Kohlmeyer <...@apache.org>
Subject Re: PR #1868: new ThreadMonitoring feature
Date Tue, 05 Jun 2018 00:12:02 GMT
Some great points Bruce!!

I believe this whole thread Monitoring feature needs to have the scope 
expanded... I mean.. We would either require compensatory events to 
rollback ANYTHING that the thread had been doing until the point it was 
killed.

When it was proposed, I understood it as a mechanism that would let us 
know if there are threads that are stuck, in real time.... Seems this PR 
has some remediation build it, which would require A LOT more thought!!!

--Udo


On 6/4/18 13:55, Bruce Schuchardt wrote:
> This looks like a dangerous change that is putting a fixed time limit 
> on execution of arbitrary jobs with no real check to see if threads 
> are stuck or not.  If a thread is doing some heavy lifting it could be 
> killed with Thread.stop() in the middle of its work. Think initial 
> image transfer, for instance.  This can leave "sender" threads stuck 
> waiting for replies until they, too, are killed.
>
> No-one is even supposed to /use/ Thread.stop().
>
>
> On 6/4/18 9:50 AM, Kirk Lund wrote:
>> Can a couple more folks please review PR #1868?
>>
>> My review feedback pertained only to using dependency injection 
>> instead of
>> a singleton or static getter and Yossi has updated the code as I 
>> requested.
>> My review doesn't really address the usefulness of the feature or the
>> impact on features that were updated to use ThreadMonitoring.
>>
>> Note that there is new User API and Configuration.
>>
>> It would be good for people who work with Queues, Pools, Configuration,
>> FunctionExecutor, DistributionManager (and its Executors), Client/Server
>> communication, WAN Gateways to review this PR since it touches all of 
>> these
>> classes:
>>
>> AbstractGatewaySenderEventProcessor.java
>> AcceptorImpl.java
>> ClusterDistributionManager.java
>> ConcurrentParallelGatewaySenderEventProcessor.java
>> ConcurrentSerialGatewaySenderEventProcessor.java
>> DistributionConfig.java
>> DistributionConfigImpl.java
>> DistributionManager.java
>> ExpiryTask.java
>> FunctionExecutionPooledExecutor.java
>> GemFireCacheImpl.java
>> LonerDistributionManager.java
>> OneTaskOnlyExecutor.java
>> ParallelAsyncEventQueueImpl.java
>> ParallelGatewaySenderEventProcessor.java
>> ParallelGatewaySenderHelper.java
>> ParallelGatewaySenderImpl.java
>> PoolImpl.java
>> PRHARedundancyProvider.java
>> RemoteConcurrentParallelGatewaySenderEventProcessor.java
>> RemoteConcurrentSerialGatewaySenderEventProcessor.java
>> RemoteParallelGatewaySenderEventProcessor.java
>> RemoteSerialGatewaySenderEventProcessor.java
>> ScheduledThreadPoolExecutorWithKeepAlive.java
>> SerialAsyncEventQueueImpl.java
>> SerialGatewaySenderEventProcessor.java
>> SerialGatewaySenderImpl.java
>> SerialQueuedExecutorWithDMStats.java
>> TcpServer.java
>>
>> This PR also introduces new user visible API and configuration options:
>>
>> ConfigurationProperties.java
>> ThreadMonitoring.java
>>
>> (I may have left out a couple classes)
>>
>> https://github.com/apache/geode/pull/1868
>>
>
>


Mime
View raw message