diff --git a/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/DataUpdateServiceDefault.java b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/DataUpdateServiceDefault.java index dc2aabe8..007b2121 100644 --- a/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/DataUpdateServiceDefault.java +++ b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/DataUpdateServiceDefault.java @@ -244,7 +244,7 @@ public void run() { checkForUpdate(dataFile, false); } }, - getInterval(dataFile.getConfiguration()))); + getIntervalMillis(dataFile.getConfiguration()))); } } } @@ -435,11 +435,12 @@ public void registerDataFile(final AspectEngineDataFile dataFile) { synchronized (futureLock) { if (aspectDataFile.getAutomaticUpdatesEnabled() && aspectDataFile.getFuture() == null) { - long delay = getInterval(aspectDataFile.getConfiguration()); + long delayMillis = + getIntervalMillis(aspectDataFile.getConfiguration()); if (aspectDataFile.getUpdateAvailableTime() != null) { if (aspectDataFile.getUpdateAvailableTime().getTime() > System.currentTimeMillis()) { - delay = applyIntervalRandomisation( + delayMillis = applyIntervalRandomisationMillis( aspectDataFile.getUpdateAvailableTime().getTime() - System.currentTimeMillis(), aspectDataFile.getConfiguration()); @@ -447,7 +448,8 @@ public void registerDataFile(final AspectEngineDataFile dataFile) { } // Create a timer that will go off when the engine expects // updated data to be available. - logger.debug("Scheduling update check with delay {}", delay); + logger.debug("Scheduling update check with delay {} millis", + delayMillis); try { ScheduledFuture future = futureFactory.schedule( new Runnable() { @@ -459,7 +461,7 @@ public void run() { logger.debug("Returning from checkForUpdate"); } }, - delay); + delayMillis); logger.debug("Update check scheduled for {} millis", future.getDelay(TimeUnit.MILLISECONDS)); aspectDataFile.setFuture(future); @@ -566,35 +568,37 @@ public void unregisterDataFile(AspectEngineDataFile dataFile) { } /** - * Get the random delay in milliseconds to add on to the scheduled time - * for a specific Engine, no greater than the Engine's max randomisation. + * Get the polling interval for a data file plus a random number of + * seconds, no greater than the configured maximum randomisation. * - * @param config to get the delay for - * @return a random delay in milliseconds. + * @param config to get the interval for + * @return the interval in milliseconds. */ - private long getInterval(DataFileConfiguration config) { - int seconds = 0; + private long getIntervalMillis(DataFileConfiguration config) { + long intervalMillis = 0; if (config.getPollingIntervalSeconds() > 0) { - seconds = config.getPollingIntervalSeconds(); + intervalMillis = config.getPollingIntervalSeconds() * 1000L; } - return applyIntervalRandomisation(seconds, config) * 1000; - + return applyIntervalRandomisationMillis(intervalMillis, config); } /** - * Add a random amount of time to the specified interval + * Add a random number of whole seconds, up to the maximum in the + * configuration, to the specified interval. * - * @param interval the interval to add a random amount of time to - * @param config the {@link DataFileConfiguration} object that specifies the - * maximum number of seconds to add - * @return the new interval + * @param intervalMillis the interval in milliseconds to add to + * @param config the {@link DataFileConfiguration} object that + * specifies the maximum number of seconds to add + * @return the new interval in milliseconds */ - private long applyIntervalRandomisation(long interval, DataFileConfiguration config) { - long seconds = 0; + private long applyIntervalRandomisationMillis( + long intervalMillis, + DataFileConfiguration config) { + long randomSeconds = 0; if (config.getMaxRandomisationSeconds() > 0) { - seconds = random.nextInt(config.getMaxRandomisationSeconds()); + randomSeconds = random.nextInt(config.getMaxRandomisationSeconds()); } - return interval + seconds; + return intervalMillis + randomSeconds * 1000; } /** diff --git a/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/DataUpdateServiceTests.java b/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/DataUpdateServiceTests.java index d8768cf6..52176409 100644 --- a/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/DataUpdateServiceTests.java +++ b/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/DataUpdateServiceTests.java @@ -42,6 +42,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.internal.matchers.LessOrEqual; import org.mockito.invocation.InvocationOnMock; @@ -233,6 +234,46 @@ public boolean matches(Long aLong) { ); } + @Test + public void DataUpdateService_Register_UpdateAvailableRandomDelayInSeconds() { + // Arrange + final int registrations = 50; + final long availableInMillis = 3600000; + final long maxDelayMillis = 600 * 1000; + final Date availableAt = + new Date(System.currentTimeMillis() + availableInMillis); + DataFileConfigurationDefault config = new DataFileConfigurationDefault(); + config.setAutomaticUpdatesEnabled(true); + config.setMaxRandomisationSeconds(600); + + // Act + for (int i = 0; i < registrations; i++) { + AspectEngineDataFileDefault file = new AspectEngineDataFileDefault(); + file.setUpdateAvailableTime(availableAt); + file.setEngine(mock(OnPremiseAspectEngine.class)); + file.setConfiguration(config); + dataUpdate.registerDataFile(file); + } + + // Assert + // The delay is the time until the update is available plus a random + // number of seconds, so across many registrations at least one adds + // more than a second, and none adds more than the maximum. + ArgumentCaptor delays = ArgumentCaptor.forClass(Long.class); + verify(futureFactory, times(registrations)).schedule( + any(Runnable.class), + delays.capture()); + long largestExtra = Long.MIN_VALUE; + for (long delay : delays.getAllValues()) { + long extra = delay - availableInMillis; + largestExtra = Math.max(largestExtra, extra); + assertTrue("Delay " + delay + " adds more than the maximum", + extra <= maxDelayMillis); + } + assertTrue("Largest random delay was " + largestExtra + " millis", + largestExtra > 1000); + } + @Test public void DataUpdateService_Register_AutoUpdateConfiguredInterval() { // Arrange