[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16681802#comment-16681802
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 3b2ff449f057c20166470c41da07aa274d2ae2ad in geode's branch 
refs/heads/develop from [~bschuchardt]
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=3b2ff44 ]

GEODE-3416 Reduce synchronization blockages in SocketCreator

Fixing LGTM complaint about synch on auto-boxed variable


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
>Priority: Major
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16137047#comment-16137047
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit be4551191a17b9f50169f375798785ab5373e975 in geode's branch 
refs/heads/develop from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=be45511 ]

GEODE-3416: Reduce synchronization blockages in SocketCloser. This now closes 
#702
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16137044#comment-16137044
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/702


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16137040#comment-16137040
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 26dcf43647117c1729cc7a7f010de2774785c183 in geode's branch 
refs/heads/feature/GEODE-3416 from [~dbarnes97]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=26dcf43 ]

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16135894#comment-16135894
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user bschuchardt commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r134339691
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

@kohlmu-pivotal I don't believe that I said it was different behavior.  I 
said it needs extensive testing.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16135833#comment-16135833
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit c6b20a91f701315639b12458f404075395016c87 in geode's branch 
refs/heads/feature/GEODE-3416 from [~dbarnes97]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=c6b20a9 ]

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16130916#comment-16130916
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit c20ac681d3026aaf04a1219c9c80ce35e106e766 in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=c20ac68 ]

GEODE-3416: Fixed the creation of a threadPool for non JDK1.8


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16129074#comment-16129074
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user hiteshk25 commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133506959
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

Thanks Bruce. So we close socket closer after closing all the connections? 
So we just have a executor in sync(but socket close always happens in another 
thread).


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16129068#comment-16129068
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit e7c3fb21ee690cd9b8bbdb9a448b3b2a08dd699d in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=e7c3fb2 ]

GEODE-3416: Revert initial change


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16128269#comment-16128269
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133353995
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java ---
@@ -929,10 +929,6 @@ protected void removeEndpoint(DistributedMember 
memberID, String reason,
   owner.getDM().getMembershipManager().getShutdownCause());
 }
   }
-
-  if (remoteAddress != null) {
-
this.socketCloser.releaseResourcesForAddress(remoteAddress.toString());
-  }
--- End diff --

it does a missed revert...


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127649#comment-16127649
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133252327
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,56 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
--- End diff --

I always appreciate it as a consumer if deprecations contain information 
about what the new solution is for the problem that the deprecated code was 
solving.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127621#comment-16127621
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133255222
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

Correct and agreed. When the cache closes, I imagine SocketCloser.close() 
is called. Which means the closing of any socket post that would be done 
in-line. 
I don't know what the expected behavior is supposed to be, for the closing 
of the sockets when that cache is closing. But you are correct that it could 
mean that the closing of the socket could be holding up the shutting down of 
the cache.



> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the 

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127594#comment-16127594
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user hiteshk25 commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133249141
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java ---
@@ -929,10 +929,6 @@ protected void removeEndpoint(DistributedMember 
memberID, String reason,
   owner.getDM().getMembershipManager().getShutdownCause());
 }
   }
-
-  if (remoteAddress != null) {
-
this.socketCloser.releaseResourcesForAddress(remoteAddress.toString());
-  }
--- End diff --

This doesn't require??


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127552#comment-16127552
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 2caddb0dc71649e801bf7c3e763782c891de7317 in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=2caddb0 ]

GEODE-3416: spotlessApply


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127507#comment-16127507
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 170e333a9f07c74297d0875ef244de118a656bd0 in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=170e333 ]

GEODE-3416: Moved the clearing of the map to be outside of the loop
in the close() method
Amended the threadPoolExecutor to set the correct thread timeout. Using
th asyncClosePoolKeepAlive instead of asyncCloseWait.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127501#comment-16127501
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133238599
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

ExecutorService.shutdown does protect itself internally with locks, etc.. 
so we don't have to worry about multiple threads calling shutdown on the same 
executor service


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of 

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127500#comment-16127500
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133238417
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -144,35 +156,22 @@ private boolean isClosed() {
* called then the asyncClose will be done synchronously.
*/
   public void close() {
-synchronized (asyncCloseExecutors) {
+synchronized (closed) {
   if (!this.closed) {
 this.closed = true;
-for (ThreadPoolExecutor pool : asyncCloseExecutors.values()) {
-  pool.shutdown();
-}
-asyncCloseExecutors.clear();
+  } else {
+return;
   }
 }
+for (ExecutorService executorService : asyncCloseExecutors.values()) {
+  executorService.shutdown();
+  asyncCloseExecutors.clear();
--- End diff --

Good catch.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127441#comment-16127441
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133230527
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

This could still race with a thread running the close() method below and 
both could end up calling shutdown on the same service.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by 

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16127442#comment-16127442
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133230687
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -144,35 +156,22 @@ private boolean isClosed() {
* called then the asyncClose will be done synchronously.
*/
   public void close() {
-synchronized (asyncCloseExecutors) {
+synchronized (closed) {
   if (!this.closed) {
 this.closed = true;
-for (ThreadPoolExecutor pool : asyncCloseExecutors.values()) {
-  pool.shutdown();
-}
-asyncCloseExecutors.clear();
+  } else {
+return;
   }
 }
+for (ExecutorService executorService : asyncCloseExecutors.values()) {
+  executorService.shutdown();
+  asyncCloseExecutors.clear();
--- End diff --

Shouldn't this be outside the for loop?


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16126780#comment-16126780
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133113689
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

given that remove is on the concurrent hashmap, one should only ever get 
into this once. This should not suffer reentrancy problems.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent 

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16126672#comment-16126672
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133094233
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

Is this call (excutorService.shutdown) reentrant?


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125193#comment-16125193
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r132869478
  
--- Diff: geode-book/master_middleman/bookbinder_helpers.rb ---
@@ -1,298 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
--- End diff --

Why does Github think that this is gone? If I pull your fork and `git diff 
origin/develop...origin/feature/GEODE-3416`, I don't see any change to this 
file.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123669#comment-16123669
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit e6b25052b0bea117fc53ec9bc32bde00fc952d67 in geode's branch 
refs/heads/feature/GEODE-3416 from [~dbarnes97]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=e6b2505 ]

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123619#comment-16123619
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit ea2fc8242d2afedcf8715a6d7afc6140b8729631 in geode's branch 
refs/heads/feature/GEODE-3416 from [~dbarnes97]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=ea2fc82 ]

GEODE-3396 pub-tools support for product name & version variables, delete local 
Bookbinder helpers.

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123620#comment-16123620
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 821e03dc6f45a6a9a4c43045dfbb235a626c2ec7 in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=821e03d ]

GEODE-3416: Cleanup SocketCloser code to reduce the synchronization
Old code would force single threaded behavior, new code will synchronize
on the checking or changing or the closed flag


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123602#comment-16123602
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 5d567b1f3712127a44b3f1b556080d054423bd94 in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=5d567b1 ]

GEODE-3416: Cleanup SocketCloser code to reduce the synchronization
Old code would force single threaded behavior, new code will synchronize
on the checking or changing or the closed flag


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123601#comment-16123601
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 31e4db0f7630c4350519c29f2ce4061fc0c2b1ee in geode's branch 
refs/heads/feature/GEODE-3416 from [~dbarnes97]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=31e4db0 ]

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16120712#comment-16120712
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on the issue:

https://github.com/apache/geode/pull/702
  
@bschuchardt @dschneider-pivotal is on the review of this PR.


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16120674#comment-16120674
 ] 

ASF subversion and git services commented on GEODE-3416:


Commit 6d899bdb45047a2d9d34c22fa544712f71fb7c3b in geode's branch 
refs/heads/feature/GEODE-3416 from [~ukohlmeyer]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=6d899bd ]

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16120668#comment-16120668
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

Github user kohlmu-pivotal commented on the issue:

https://github.com/apache/geode/pull/702
  
@bschuchardt @galen-pivotal @hiteshk25 @pivotal-amurmann 
@dschneider-pivotal @WireBaron @kirklund 


> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3416) Reduce blocking for SocketCloser.asyncClose

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16120666#comment-16120666
 ] 

ASF GitHub Bot commented on GEODE-3416:
---

GitHub user kohlmu-pivotal opened a pull request:

https://github.com/apache/geode/pull/702

GEODE-3416: Reduce synchronization blockages in SocketCloser.

Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [ ] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to d...@geode.apache.org.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/geode feature/GEODE-3416

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/702.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #702


commit 56d7707e12ecdecbc1881a9ac1575fac854e8fe9
Author: Udo Kohlmeyer 
Date:   2017-08-09T21:17:59Z

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added




> Reduce blocking for SocketCloser.asyncClose 
> 
>
> Key: GEODE-3416
> URL: https://issues.apache.org/jira/browse/GEODE-3416
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.1.0, 1.1.1, 1.2.0, 1.2.1
>Reporter: Udo Kohlmeyer
>Assignee: Udo Kohlmeyer
> Fix For: 1.3.0
>
>
> In the SocketCloser.asyncClose method, there is a synchronization block 
> around at HashMap. This synchronization will cause an effective 
> single-threaded processing capability when closing sockets. this effect 
> becomes more evident with a high number of clients.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)