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

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

                Author: ASF GitHub Bot
            Created on: 27/Oct/20 16:51
            Start Date: 27/Oct/20 16:51
    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_r512860312



##########
File path: 
artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQScheduledComponent.java
##########
@@ -287,35 +295,47 @@ private void restartIfNeeded() {
       }
    }
 
-   final Runnable runForExecutor = new Runnable() {
-      @Override
-      public void run() {
-         if (onDemand && delayed.get() > 0) {
-            delayed.decrementAndGet();
-         }
+   private void runForExecutor(AtomicBoolean booked) {
+      // It unblocks:
+      // - a new delay request
+      // - next periodic run request (in case of executor != null)
+      // Although tempting, don't move this one after 
ActiveMQScheduledComponent.this.run():
+      // - it can cause "delay" to change semantic ie a racing delay while 
finished executing the task, won't succeed
+      // - it won't prevent "slow tasks" to accumulate, because slowness 
cannot be measured inside running method;
+      //   it just cause skipping runs for perfectly timed executions too
+      // Re the code as it is: if the next periodic run has been able to 
request a new round while
+      // the previous one hasn't yet started, it makes sense to skip it, 
because its job will be executed by
+      // the old one (yet to start).

Review comment:
       Yep effectively is not the right place for this bits of code, but is 
more the overall logic...I can put it in the header of the class docs!




----------------------------------------------------------------
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:
[email protected]


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

    Worklog Id:     (was: 505311)
    Time Spent: 2h 40m  (was: 2.5h)

> 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
>            Assignee: Francesco Nigro
>            Priority: Major
>          Time Spent: 2h 40m
>  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