diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c883f79..5e1f163c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### ⚙️ Technical +* Improvements to `Timer` to avoid extra executions when primary instance changes. + * Updated `ClusterService` to use Hoist's `InstanceNotFoundException` class to designate routine. * Exposed `/xh/ping` as whitelisted route for basic uptime/reachability checks. Retained legacy diff --git a/grails-app/controllers/io/xh/hoist/admin/cluster/LogViewerAdminController.groovy b/grails-app/controllers/io/xh/hoist/admin/cluster/LogViewerAdminController.groovy index 38b0fc8d..ac410e1b 100644 --- a/grails-app/controllers/io/xh/hoist/admin/cluster/LogViewerAdminController.groovy +++ b/grails-app/controllers/io/xh/hoist/admin/cluster/LogViewerAdminController.groovy @@ -11,6 +11,7 @@ import groovy.io.FileType import io.xh.hoist.BaseController import io.xh.hoist.cluster.ClusterRequest import io.xh.hoist.configuration.LogbackConfig +import io.xh.hoist.exception.RoutineRuntimeException import io.xh.hoist.security.Access import static io.xh.hoist.util.Utils.getAppContext @@ -67,13 +68,12 @@ class LogViewerAdminController extends BaseController { def doCall() { if (!availableFiles[filename]) throwUnavailable() - // Catch any exceptions and render clean failure - the admin client auto-polls for log file - // updates, and we don't want to spam the logs with a repeated stacktrace. + // Wrap any exceptions as routine as the admin client aggressively auto-polls. try { def content = appContext.logReaderService.readFile(filename, startLine, maxLines, pattern, caseSensitive) return [success: true, filename: filename, content: content] } catch (Exception e) { - return [success: false, filename: filename, content: [], exception: e.message] + throw new RoutineRuntimeException(e.message) } } } diff --git a/grails-app/services/io/xh/hoist/alertbanner/AlertBannerService.groovy b/grails-app/services/io/xh/hoist/alertbanner/AlertBannerService.groovy index 88795e74..f369af57 100644 --- a/grails-app/services/io/xh/hoist/alertbanner/AlertBannerService.groovy +++ b/grails-app/services/io/xh/hoist/alertbanner/AlertBannerService.groovy @@ -51,6 +51,7 @@ class AlertBannerService extends BaseService { void init() { timer = createTimer( + name: 'readFromSpec', interval: 2 * MINUTES, runFn: this.&readFromSpec, primaryOnly: true diff --git a/grails-app/services/io/xh/hoist/clienterror/ClientErrorService.groovy b/grails-app/services/io/xh/hoist/clienterror/ClientErrorService.groovy index 808ee14d..d6e43dda 100644 --- a/grails-app/services/io/xh/hoist/clienterror/ClientErrorService.groovy +++ b/grails-app/services/io/xh/hoist/clienterror/ClientErrorService.groovy @@ -48,6 +48,7 @@ class ClientErrorService extends BaseService { void init() { super.init() createTimer( + name: 'processErrors', interval: { alertInterval }, delay: 15 * SECONDS, primaryOnly: true diff --git a/grails-app/services/io/xh/hoist/feedback/FeedbackEmailService.groovy b/grails-app/services/io/xh/hoist/feedback/FeedbackEmailService.groovy index 597bade4..c7ed4c77 100644 --- a/grails-app/services/io/xh/hoist/feedback/FeedbackEmailService.groovy +++ b/grails-app/services/io/xh/hoist/feedback/FeedbackEmailService.groovy @@ -16,6 +16,7 @@ class FeedbackEmailService extends BaseService { void init() { subscribeToTopic( + name: 'emailFeedback', topic: 'xhFeedbackReceived', onMessage: this.&emailFeedback, primaryOnly: true diff --git a/grails-app/services/io/xh/hoist/monitor/MonitorService.groovy b/grails-app/services/io/xh/hoist/monitor/MonitorService.groovy index 7208e4d5..aeebabd0 100644 --- a/grails-app/services/io/xh/hoist/monitor/MonitorService.groovy +++ b/grails-app/services/io/xh/hoist/monitor/MonitorService.groovy @@ -51,6 +51,7 @@ class MonitorService extends BaseService { void init() { timer = createTimer( + name: 'runMonitors', interval: { monitorInterval }, delay: startupDelay, primaryOnly: true diff --git a/src/main/groovy/io/xh/hoist/cache/Cache.groovy b/src/main/groovy/io/xh/hoist/cache/Cache.groovy index 26aa17c3..68343c1b 100644 --- a/src/main/groovy/io/xh/hoist/cache/Cache.groovy +++ b/src/main/groovy/io/xh/hoist/cache/Cache.groovy @@ -51,6 +51,7 @@ class Cache extends BaseCache { if (onChange) addChangeHandler(onChange) timer = new Timer( + name: name ? "cullEntries_$name" : 'cullEntries', owner: svc, primaryOnly: replicate, runFn: this.&cullEntries, diff --git a/src/main/groovy/io/xh/hoist/role/provided/DefaultRoleService.groovy b/src/main/groovy/io/xh/hoist/role/provided/DefaultRoleService.groovy index c9e10383..605d94be 100644 --- a/src/main/groovy/io/xh/hoist/role/provided/DefaultRoleService.groovy +++ b/src/main/groovy/io/xh/hoist/role/provided/DefaultRoleService.groovy @@ -102,6 +102,7 @@ class DefaultRoleService extends BaseRoleService { ensureRequiredConfigAndRolesCreated() timer = createTimer( + name: 'refreshRoles', interval: { config.refreshIntervalSecs as int * SECONDS }, runFn: this.&refreshRoleAssignments, runImmediatelyAndBlock: true, diff --git a/src/main/groovy/io/xh/hoist/util/Timer.groovy b/src/main/groovy/io/xh/hoist/util/Timer.groovy index b46c7598..d4c6c446 100644 --- a/src/main/groovy/io/xh/hoist/util/Timer.groovy +++ b/src/main/groovy/io/xh/hoist/util/Timer.groovy @@ -7,6 +7,8 @@ package io.xh.hoist.util +import io.xh.hoist.BaseService +import io.xh.hoist.cache.CachedValue import io.xh.hoist.log.LogSupport import java.util.concurrent.ExecutionException @@ -31,10 +33,10 @@ class Timer { private static Long CONFIG_INTERVAL = 15 * SECONDS - /** Optional name for this timer (for logging purposes) **/ + /** Unique name for this timer, required for cluster aware timers (see `primaryOnly`) **/ final String name - /** Object using this timer (for logging purposes) **/ + /** Object using this timer **/ final LogSupport owner /** Closure to run */ @@ -73,7 +75,10 @@ class Timer { /** Block on an immediate initial run? Default is false. */ final boolean runImmediatelyAndBlock - /** Only run job when clustered instance is the primary instance? Default is false. */ + /** + * Only run job when clustered instance is the primary instance? Default is false. + * For timers owned by instances of BaseService only. + */ final boolean primaryOnly @@ -109,6 +114,9 @@ class Timer { private java.util.Timer configTimer + private CachedValue _lastCompletedOnCluster + + // Args from Grails 3.0 async promise implementation static ExecutorService executorService = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue()) @@ -132,6 +140,16 @@ class Timer { if ([owner, interval, runFn].contains(null)) throw new RuntimeException('Missing required arguments for Timer.') if (config.delayUnits) throw new RuntimeException('delayUnits has been removed from the API. Specify delay in ms.') + if (primaryOnly) { + if (!name) { + throw new IllegalArgumentException("Cannot create a 'primaryOnly' timer without a unique name") + } + if (!owner instanceof BaseService) { + throw new IllegalArgumentException("A 'primaryOnly' timer must be owned by an instance of BaseService.") + } + _lastCompletedOnCluster = new CachedValue<>(name: "xhTimer_$name", svc: owner as BaseService) + } + intervalMs = calcIntervalMs() timeoutMs = calcTimeoutMs() delayMs = calcDelayMs() @@ -217,6 +235,7 @@ class Timer { } _lastRunCompleted = new Date() + _lastCompletedOnCluster?.set(_lastRunCompleted) _isRunning = false _lastRunStats = [ startTime: _lastRunStarted, @@ -287,15 +306,19 @@ class Timer { // frequently enough to pickup forceRun reasonably fast. Tighten down for the rare fast timer. //------------------------------------------------------------------------------------------- private void onCoreTimer() { - if (!isRunning) { - if ((intervalMs > 0 && intervalElapsed(intervalMs, lastRunCompleted)) || forceRun) { - boolean wasForced = forceRun - doRun() - if (wasForced) forceRun = false - } + if (!isRunning && (forceRun || isIntervalElapsed())) { + boolean wasForced = forceRun + doRun() + if (wasForced) forceRun = false } } + private boolean isIntervalElapsed() { + if (intervalMs <= 0) return false + def lastRun = _lastCompletedOnCluster ? _lastCompletedOnCluster.get() : _lastRunCompleted + return intervalElapsed(intervalMs, lastRun) + } + private Long calcCoreIntervalMs() { return (intervalMs > 2 * SECONDS) ? 1 * SECONDS : 250; }