From 9ea0573bfcfe246b3f6f1731ad49274890db80b5 Mon Sep 17 00:00:00 2001 From: Jonathan Gamba Date: Tue, 15 Oct 2024 11:04:58 -0600 Subject: [PATCH] Fix (Core) Fixed calculateBackoffTime in JobQueueManagerAPIImpl (#30350) This pull request introduces enhancements to the job queue management system to handle empty queue scenarios more effectively. The key changes include adding constants to cap and reset the empty queue count, refactoring the backoff time calculation, and adding tests for the new functionality. Enhancements to job queue management: * [`dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java`](diffhunk://#diff-c092f8af2f800c0ca84f2c2f4aed0af60d0ed488e20fa32992967e154064f561R117-R121): Introduced `MAX_EMPTY_QUEUE_COUNT` and `EMPTY_QUEUE_RESET_THRESHOLD` constants to cap and reset the empty queue count. * [`dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java`](diffhunk://#diff-c092f8af2f800c0ca84f2c2f4aed0af60d0ed488e20fa32992967e154064f561L476-R485): Refactored `processJobs` method to use `calculateBackoffTime` and `incrementAndResetEmptyQueueCount` methods for better handling of empty queue scenarios. * [`dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java`](diffhunk://#diff-c092f8af2f800c0ca84f2c2f4aed0af60d0ed488e20fa32992967e154064f561R975-R1005): Added `calculateBackoffTime` and `incrementAndResetEmptyQueueCount` methods to encapsulate the logic for calculating backoff time and resetting the empty queue count. Testing improvements: * [`dotcms-integration/src/test/java/com/dotcms/jobs/business/api/JobQueueManagerAPITest.java`](diffhunk://#diff-93563af6a56c7a961561131bb9cb95c6ea24b9ba8508e3a98dc49d036ff78859R1293-R1312): Added `test_calculateBackoffTime` method to verify the correctness of the `calculateBackoffTime` method under various scenarios. --- .../business/api/JobQueueManagerAPIImpl.java | 42 ++++++++++++++++++- .../business/api/JobQueueManagerAPITest.java | 20 +++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java index f3b4186b1af7..d45b26b8b476 100644 --- a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java @@ -114,6 +114,11 @@ public class JobQueueManagerAPIImpl implements JobQueueManagerAPI { private final EventProducer eventProducer; private final JobProcessorFactory jobProcessorFactory; + // Cap to prevent overflow + private static final int MAX_EMPTY_QUEUE_COUNT = 30; + // Arbitrary threshold to reset + private static final int EMPTY_QUEUE_RESET_THRESHOLD = Integer.MAX_VALUE - 1000; + /** * Constructs a new JobQueueManagerAPIImpl. * This constructor initializes the job queue manager with all necessary dependencies and configurations. @@ -473,9 +478,11 @@ private void processJobs() { } else { // If no jobs were found, wait for a short time before checking again // Implement exponential backoff when queue is repeatedly empty - long sleepTime = Math.min(1000 * (long) Math.pow(2, emptyQueueCount), 30000); + long sleepTime = calculateBackoffTime(emptyQueueCount, MAX_EMPTY_QUEUE_COUNT); Thread.sleep(sleepTime); - emptyQueueCount++; + emptyQueueCount = incrementAndResetEmptyQueueCount( + emptyQueueCount, MAX_EMPTY_QUEUE_COUNT, EMPTY_QUEUE_RESET_THRESHOLD + ); } } catch (InterruptedException e) { Logger.error(this, "Job processing thread interrupted: " + e.getMessage(), e); @@ -965,6 +972,37 @@ private float getJobProgress(final Job job) { return progress; } + /** + * Calculates the backoff time based on the number of empty queue counts. + * + * @param emptyQueueCount the current count of empty queue checks + * @param maxEmptyQueueCount the maximum count of empty queue checks + * @return the calculated backoff time in milliseconds, the result is capped at 30,000 + * milliseconds (30 seconds) to prevent excessively long sleep times. + */ + @VisibleForTesting + public long calculateBackoffTime(int emptyQueueCount, int maxEmptyQueueCount) { + emptyQueueCount = Math.min(emptyQueueCount, maxEmptyQueueCount); + return Math.min(1000L * (1L << emptyQueueCount), 30000L); + } + + /** + * Increments the empty queue count and resets it if it exceeds the reset threshold. + * + * @param emptyQueueCount the current count of empty queue checks + * @param maxEmptyQueueCount the maximum count of empty queue checks + * @param resetThreshold the threshold at which the empty queue count should be reset + * @return the updated empty queue count + */ + private int incrementAndResetEmptyQueueCount( + int emptyQueueCount, int maxEmptyQueueCount, int resetThreshold) { + emptyQueueCount++; + if (emptyQueueCount > resetThreshold) { + emptyQueueCount = maxEmptyQueueCount; // Reset to max to avoid wrap around + } + return emptyQueueCount; + } + /** * A wrapper class that makes ScheduledExecutorService auto-closeable. This class is designed to * be used with try-with-resources to ensure that the ScheduledExecutorService is properly shut diff --git a/dotcms-integration/src/test/java/com/dotcms/jobs/business/api/JobQueueManagerAPITest.java b/dotcms-integration/src/test/java/com/dotcms/jobs/business/api/JobQueueManagerAPITest.java index 28d1e137bbbc..1ea8589cc6c4 100644 --- a/dotcms-integration/src/test/java/com/dotcms/jobs/business/api/JobQueueManagerAPITest.java +++ b/dotcms-integration/src/test/java/com/dotcms/jobs/business/api/JobQueueManagerAPITest.java @@ -1290,6 +1290,26 @@ public void test_complex_cancelJob() throws Exception { jobQueueManagerAPI.close(); } + /** + * Method to test: calculateBackoffTime in JobQueueManagerAPI + * Given Scenario: Various empty queue counts and maximum empty queue count + * ExpectedResult: Correct backoff times are calculated + */ + @Test + public void test_calculateBackoffTime() { + + JobQueueManagerAPIImpl jobQueueManager = (JobQueueManagerAPIImpl) jobQueueManagerAPI; + + assertEquals(1000L, jobQueueManager.calculateBackoffTime(0, 30)); + assertEquals(2000L, jobQueueManager.calculateBackoffTime(1, 30)); + assertEquals(4000L, jobQueueManager.calculateBackoffTime(2, 30)); + assertEquals(8000L, jobQueueManager.calculateBackoffTime(3, 30)); + assertEquals(16000L, jobQueueManager.calculateBackoffTime(4, 30)); + assertEquals(30000L, jobQueueManager.calculateBackoffTime(5, 30)); + assertEquals(30000L, jobQueueManager.calculateBackoffTime(6, 30)); + assertEquals(30000L, jobQueueManager.calculateBackoffTime(30, 30)); + } + /** * Creates a new instance of the JobQueueManagerAPI with the provided configurations. *