Skip to content

Add the random delay before an available update as seconds - #74

Draft
jwrosewell wants to merge 2 commits into
fix/data-update-schedule-racefrom
fix/random-delay-units
Draft

jwrosewell wants to merge 2 commits into
fix/data-update-schedule-racefrom
fix/random-delay-units

Conversation

@jwrosewell

Copy link
Copy Markdown
Contributor

For #72

This pull request is based on #71's branch (fix/data-update-schedule-race) because both change DataUpdateServiceDefault.java. It must be retargeted to rc if #71 merges first.

Problem

When a data file says when its next update is available, the data update service schedules the next check for that time plus a random delay of up to MaxRandomisationSeconds seconds (default 600). applyIntervalRandomisation added the random seconds to the interval in whatever unit it was given. getInterval passed seconds and multiplied by 1000, which was right. registerDataFile passed the milliseconds until the update is available, so the random seconds were added as milliseconds and the checks spread over under 0.6 seconds instead of 10 minutes.

Change

applyIntervalRandomisation is now applyIntervalRandomisationMillis. It takes and returns milliseconds and converts the random seconds itself. getInterval is now getIntervalMillis and returns milliseconds without a second conversion. The variable passed to the scheduler is delayMillis, and the javadoc of getIntervalMillis now describes what it returns. Its second caller, the reschedule inside checkForUpdate, already used milliseconds and is unchanged in behaviour.

Verification

New test DataUpdateService_Register_UpdateAvailableRandomDelayInSeconds registers 50 data files whose update is available in one hour, with randomisation of 600 seconds. It checks no delay exceeds the available time by more than the maximum and that the largest exceeds it by more than one second.

Release note

The random delay before checking for an available update is now spread over up to MaxRandomisationSeconds seconds, as configured.

Outstanding

Whether .NET has the same mismatch has not been checked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant