You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by franz1981 <gi...@git.apache.org> on 2017/10/13 07:16:40 UTC

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

GitHub user franz1981 opened a pull request:

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

    ARTEMIS-1462 Allow ActiveMQScheduledComponent initial delay configuration

    It contains:
     - an improved documentation of the constructors
     - the initial delay configuration

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

    $ git pull https://github.com/franz1981/activemq-artemis initial_delay

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

    https://github.com/apache/activemq-artemis/pull/1586.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 #1586
    
----
commit 06401e020b501b1fd2a45f59e2766e613407d93b
Author: Francesco Nigro <ni...@gmail.com>
Date:   2017-10-13T07:15:41Z

    ARTEMIS-1462 Allow ActiveMQScheduledComponent initial delay configuration
    
    It contains:
     - an improved documentation of the constructors
     - the initial delay configuration

----


---

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1586#discussion_r144581161
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ActiveMQScheduledComponentTest.java ---
    @@ -165,4 +165,37 @@ public void run() {
           }
        }
     
    +   @Test
    +   public void testUsingCustomInitialDelay() throws InterruptedException {
    +      final CountDownLatch latch = new CountDownLatch(1);
    +      final long initialDelayMillis = 100;
    +      final long checkPeriodMillis = 100 * initialDelayMillis;
    +      final ActiveMQScheduledComponent local = new ActiveMQScheduledComponent(scheduledExecutorService, executorService, initialDelayMillis, checkPeriodMillis, TimeUnit.MILLISECONDS, false) {
    +         @Override
    +         public void run() {
    +            latch.countDown();
    +         }
    +      };
    +      final long start = System.nanoTime();
    +      local.start();
    +      try {
    +         final boolean triggeredBeforePeriod = latch.await(local.getPeriod(), local.getTimeUnit());
    +         final long timeToFirstTrigger = TimeUnit.NANOSECONDS.convert(System.nanoTime() - start, local.getTimeUnit());
    --- End diff --
    
    do you actually use NANOSECONDS on the Lock?


---

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1586#discussion_r144591374
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ActiveMQScheduledComponentTest.java ---
    @@ -165,4 +165,37 @@ public void run() {
           }
        }
     
    +   @Test
    +   public void testUsingCustomInitialDelay() throws InterruptedException {
    +      final CountDownLatch latch = new CountDownLatch(1);
    +      final long initialDelayMillis = 100;
    +      final long checkPeriodMillis = 100 * initialDelayMillis;
    +      final ActiveMQScheduledComponent local = new ActiveMQScheduledComponent(scheduledExecutorService, executorService, initialDelayMillis, checkPeriodMillis, TimeUnit.MILLISECONDS, false) {
    +         @Override
    +         public void run() {
    +            latch.countDown();
    +         }
    +      };
    +      final long start = System.nanoTime();
    +      local.start();
    +      try {
    +         final boolean triggeredBeforePeriod = latch.await(local.getPeriod(), local.getTimeUnit());
    +         final long timeToFirstTrigger = TimeUnit.NANOSECONDS.convert(System.nanoTime() - start, local.getTimeUnit());
    --- End diff --
    
    Depends on the case but there are parts where I've used it: It is immune to ntpd/clock changes...


---

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1586#discussion_r144595987
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ActiveMQScheduledComponentTest.java ---
    @@ -165,4 +165,37 @@ public void run() {
           }
        }
     
    +   @Test
    +   public void testUsingCustomInitialDelay() throws InterruptedException {
    +      final CountDownLatch latch = new CountDownLatch(1);
    +      final long initialDelayMillis = 100;
    +      final long checkPeriodMillis = 100 * initialDelayMillis;
    +      final ActiveMQScheduledComponent local = new ActiveMQScheduledComponent(scheduledExecutorService, executorService, initialDelayMillis, checkPeriodMillis, TimeUnit.MILLISECONDS, false) {
    +         @Override
    +         public void run() {
    +            latch.countDown();
    +         }
    +      };
    +      final long start = System.nanoTime();
    +      local.start();
    +      try {
    +         final boolean triggeredBeforePeriod = latch.await(local.getPeriod(), local.getTimeUnit());
    +         final long timeToFirstTrigger = TimeUnit.NANOSECONDS.convert(System.nanoTime() - start, local.getTimeUnit());
    --- End diff --
    
    I'm not scheduling at nanos but measuring elapsed time in nanoseconds, it is different :)
    The scheduling is in seconds or milliseconds in some tests.


---

[GitHub] activemq-artemis pull request #1586: ARTEMIS-1462 Allow ActiveMQScheduledCom...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1586#discussion_r144593753
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ActiveMQScheduledComponentTest.java ---
    @@ -165,4 +165,37 @@ public void run() {
           }
        }
     
    +   @Test
    +   public void testUsingCustomInitialDelay() throws InterruptedException {
    +      final CountDownLatch latch = new CountDownLatch(1);
    +      final long initialDelayMillis = 100;
    +      final long checkPeriodMillis = 100 * initialDelayMillis;
    +      final ActiveMQScheduledComponent local = new ActiveMQScheduledComponent(scheduledExecutorService, executorService, initialDelayMillis, checkPeriodMillis, TimeUnit.MILLISECONDS, false) {
    +         @Override
    +         public void run() {
    +            latch.countDown();
    +         }
    +      };
    +      final long start = System.nanoTime();
    +      local.start();
    +      try {
    +         final boolean triggeredBeforePeriod = latch.await(local.getPeriod(), local.getTimeUnit());
    +         final long timeToFirstTrigger = TimeUnit.NANOSECONDS.convert(System.nanoTime() - start, local.getTimeUnit());
    --- End diff --
    
    where you need NANOSECONDS? scheduling at nanosecond may kill perf.. where is it? 


---