geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bruce Schuchardt <bschucha...@pivotal.io>
Subject Re: PR #1868: new ThreadMonitoring feature
Date Mon, 04 Jun 2018 20:55:12 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message