[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

clebertsuconic-3
GitHub user clebertsuconic opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1620

    ARTEMIS-1447 Reuse thread pools on JDBC Locks

    I had moved the initialization of the thread pools for that..
    I also have changed the CrticialAnalyzer to use the same pools as part of this

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

    $ git pull https://github.com/clebertsuconic/activemq-artemis pool

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

    https://github.com/apache/activemq-artemis/pull/1620.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 #1620
   
----
commit a1d5ec3c3672ee7efcb6c436021c2f037cd181f5
Author: Clebert Suconic <[hidden email]>
Date:   2017-10-27T19:14:28Z

    ARTEMIS-1447 Reuse thread pools on JDBC Locks
   
    I had moved the initialization of the thread pools for that..
    I also have changed the CrticialAnalyzer to use the same pools as part of this

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1620: ARTEMIS-1447 Reuse thread pools on JDBC Locks

clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1620
 
    My question/concern is what happens if the scheduled executor threads get locked up them selves due to other code that's sharing it's thread, it then wouldn't run to be able to see such. I think the critical analyser really should get its own thread(s)


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1620: ARTEMIS-1447 Reuse thread pools on JDBC Locks

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1620
 
    @michaelandrepearce good point... I have changed it.. (by Passing null to CriticalAnalyzerIMpl, it will use its own pool through ActiveMQScheduledComponent).


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1620#discussion_r147536740
 
    --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java ---
    @@ -31,7 +34,27 @@
     
        private volatile long timeoutNanoSeconds;
     
    -   private volatile long checkTimeNanoSeconds;
    +   // one minute by default.. the server will change it for sure
    +   private volatile long checkTimeNanoSeconds = TimeUnit.SECONDS.toNanos(60);
    +
    +   private final ActiveMQScheduledComponent scheduledComponent;
    +
    +   private final AtomicBoolean running = new AtomicBoolean(false);
    +
    +   public CriticalAnalyzerImpl() {
    +      // this will make the scheduled component to start its own pool
    +      this(null, null);
    +   }
    +
    +   public CriticalAnalyzerImpl(ScheduledExecutorService scheduledExecutorService, Executor executor) {
    --- End diff --
   
    Should this then become private? To avoid people using it withou the null's?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1620#discussion_r147543341
 
    --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java ---
    @@ -31,7 +34,27 @@
     
        private volatile long timeoutNanoSeconds;
     
    -   private volatile long checkTimeNanoSeconds;
    +   // one minute by default.. the server will change it for sure
    +   private volatile long checkTimeNanoSeconds = TimeUnit.SECONDS.toNanos(60);
    +
    +   private final ActiveMQScheduledComponent scheduledComponent;
    +
    +   private final AtomicBoolean running = new AtomicBoolean(false);
    +
    +   public CriticalAnalyzerImpl() {
    +      // this will make the scheduled component to start its own pool
    +      this(null, null);
    +   }
    +
    +   public CriticalAnalyzerImpl(ScheduledExecutorService scheduledExecutorService, Executor executor) {
    --- End diff --
   
    I can remove the constructor.. and document the intention.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1620: ARTEMIS-1447 Reuse thread pools on JDBC Locks

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1620
 
    I have removed that pool.. and added a comment.
   
   
    I think this is good.. my testsuite (full one) ran fine


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1620#discussion_r147543585
 
    --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java ---
    @@ -31,7 +34,27 @@
     
        private volatile long timeoutNanoSeconds;
     
    -   private volatile long checkTimeNanoSeconds;
    +   // one minute by default.. the server will change it for sure
    +   private volatile long checkTimeNanoSeconds = TimeUnit.SECONDS.toNanos(60);
    +
    +   private final ActiveMQScheduledComponent scheduledComponent;
    +
    +   private final AtomicBoolean running = new AtomicBoolean(false);
    +
    +   public CriticalAnalyzerImpl() {
    +      // this will make the scheduled component to start its own pool
    +      this(null, null);
    +   }
    +
    +   public CriticalAnalyzerImpl(ScheduledExecutorService scheduledExecutorService, Executor executor) {
    --- End diff --
   
    removed


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1620: ARTEMIS-1447 Reuse thread pools on JDBC...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1620


---