[ https://issues.apache.org/jira/browse/ARTEMIS-2926?focusedWorklogId=505218&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-505218 ]
ASF GitHub Bot logged work on ARTEMIS-2926: ------------------------------------------- Author: ASF GitHub Bot Created on: 27/Oct/20 13:35 Start Date: 27/Oct/20 13:35 Worklog Time Spent: 10m Work Description: gemmellr commented on a change in pull request #3287: URL: https://github.com/apache/activemq-artemis/pull/3287#discussion_r512695885 ########## 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: Actually, I see there now is a getTimeUnit, so I think this change needs effectively unwound...it should return the same value as passed in. ---------------------------------------------------------------- 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: 505218) Time Spent: 1.5h (was: 1h 20m) > 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: 1.5h > 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)