[ 
https://issues.apache.org/jira/browse/ARTEMIS-2926?focusedWorklogId=505229&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-505229
 ]

ASF GitHub Bot logged work on ARTEMIS-2926:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 27/Oct/20 13:58
            Start Date: 27/Oct/20 13:58
    Worklog Time Spent: 10m 
      Work Description: franz1981 commented on a change in pull request #3287:
URL: https://github.com/apache/activemq-artemis/pull/3287#discussion_r512714310



##########
File path: 
artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQScheduledComponent.java
##########
@@ -188,41 +202,53 @@ public ClassLoader run() {
 
    }
 
+   /**
+    * If {@code overBooking} is {@code true} a delay request can succeed only 
if:
+    * <ul>
+    *    <li>there is no other pending delay request
+    *    <li>there is no pending execution request
+    * </ul>
+    * <p>
+    * If {@code overBooking} is {@code false} a delay request must always 
succeed.<br>
+    * When a delay request succeed it schedule a new execution to happen in 
{@link #getPeriod()}.<br>
+    */
    public void delay() {
-      int value = delayed.incrementAndGet();
-      if (value > 10) {
-         delayed.decrementAndGet();
-      } else {
-         // We only schedule up to 10 periods upfront.
-         // this is to avoid a window where a current one would be running and 
a next one is coming.
-         // in theory just 2 would be enough. I'm using 10 as a precaution 
here.
-         scheduledExecutorService.schedule(runForScheduler, Math.min(period, 
period * value), timeUnit);
+      final AtomicBoolean bookedForRunning = this.bookedForRunning;
+      if (bookedForRunning == null || bookedForRunning.compareAndSet(false, 
true)) {
+         try {
+            scheduledExecutorService.schedule(() -> 
delayedRunForScheduler(bookedForRunning), nsPeriod, TimeUnit.NANOSECONDS);
+         } catch (RejectedExecutionException e) {
+            if (bookedForRunning == null) {
+               bookedForRunning.set(false);
+            }
+            throw e;
+         }
       }
    }
 
    public long getPeriod() {
-      return period;
+      return timeUnit.convert(nsPeriod, TimeUnit.NANOSECONDS);

Review comment:
       TBH given that we don't measure time anymore it's fine :) 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 505229)
    Time Spent: 1h 50m  (was: 1h 40m)

> Scheduled task executions are skipped randomly
> ----------------------------------------------
>
>                 Key: ARTEMIS-2926
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-2926
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 2.13.0
>            Reporter: Apache Dev
>            Priority: Major
>          Time Spent: 1h 50m
>  Remaining Estimate: 0h
>
> Scheduled tasks extending {{ActiveMQScheduledComponent}} could randomly skip 
> an execution, logging:
> {code}
> Execution ignored due to too many simultaneous executions, probably a 
> previous delayed execution
> {code}
> The problem is in the "ActiveMQScheduledComponent#runForExecutor" Runnable.
> Times to be compared ({{currentTimeMillis()}} and {{lastTime}}) are taken 
> inside the runnable execution itself. So, depending on relative execution 
> times, it could happen that the difference is less than the given period 
> (e.g. 1 ms), resulting in a skipped execution.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to