diff --git a/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTask.java b/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTask.java index 86dedbf..6750bff 100644 --- a/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTask.java +++ b/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTask.java @@ -23,6 +23,7 @@ import java.util.Optional; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.ScheduleRunContext; import com.storebrand.scheduledtask.ScheduledTaskRegistry.ScheduleRunnable; @@ -72,10 +73,22 @@ public interface ScheduledTask { * master lock it will delay for a short amount of time depending on the implementation. The current default * implementation will sleep for up to two minutes between checking for new tasks. *

- * This will prepend a line to the logs informing this schedule run was manually started. + * This will prepend a line to the logs informing this schedule run was {@link RunOnce#PROGRAMMATIC} started. */ void runNow(); + + /** + * Sets a schedule to run immediately. Note it will first mark this schedule to run by setting a flag in the db, + * then wake up the scheduler thread so it will be triggered, assuming this is called on the node that has the + * master lock it will trigger nearly instantly. However, if this where triggered by a node that does not have the + * master lock it will delay for a short amount of time depending on the implementation. The current default + * implementation will sleep for up to two minutes between checking for new tasks. + *

+ * This will prepend a line to the logs informing this schedule run was {@link RunOnce} started. + */ + void runNow(RunOnce runOnce); + /** * Check if the schedule task thread is alive. This should in theory always be true, but if the thread has been * stopped by some external means it will return false. diff --git a/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistry.java b/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistry.java index 61f00b3..7dcfb8a 100644 --- a/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistry.java +++ b/scheduledtask-api/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistry.java @@ -124,6 +124,29 @@ enum State { DONE } + /** + * Represents the source on what triggered the task to run outside the normal schedule. + */ + enum RunOnce { + /** + * The task where triggered by the monitor + */ + MONITOR, + /** + * The task was triggered programmatically by calling {@link ScheduledTask#runNow(RunOnce)} + */ + PROGRAMMATIC; + + public static RunOnce fromString(String text) { + for (RunOnce s : RunOnce.values()) { + if (s.name().equalsIgnoreCase(text)) { + return s; + } + } + return null; + } + } + /** * Interface that all tasks are required to implement. Contains a run method that should perform the actual task. */ @@ -269,6 +292,11 @@ interface Schedule { */ boolean isRunOnce(); + /** + * Check what triggered the runOnce flagg + */ + Optional getRunOnce(); + /** * If set informs that this schedule has a new cron expression that differs from the one defined in the code. */ diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistryImpl.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistryImpl.java index 951d836..4483319 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistryImpl.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRegistryImpl.java @@ -18,10 +18,10 @@ import static java.util.stream.Collectors.toMap; -import java.sql.Timestamp; import java.time.Clock; import java.time.Instant; import java.time.LocalDateTime; +import java.time.ZoneId; import java.time.temporal.ChronoUnit; import java.util.List; import java.util.Map; @@ -449,12 +449,12 @@ void stop() { public static class ScheduleImpl implements Schedule { private final String scheduleName; private final boolean active; - private final boolean runOnce; + private final RunOnce runOnce; private final String overriddenCronExpression; private final Instant nextRun; private final Instant lastUpdated; - public ScheduleImpl(String scheduleName, boolean active, boolean runOnce, String cronExpression, + public ScheduleImpl(String scheduleName, boolean active, RunOnce runOnce, String cronExpression, Instant nextRun, Instant lastUpdated) { this.scheduleName = scheduleName; this.active = active; @@ -476,9 +476,13 @@ public boolean isActive() { @Override public boolean isRunOnce() { - return runOnce; + return getRunOnce().isPresent(); } + @Override + public Optional getRunOnce() { + return Optional.ofNullable(runOnce); + } @Override public Optional getOverriddenCronExpression() { @@ -506,12 +510,12 @@ public static class LogEntryImpl implements LogEntry { private final String _stackTrace; private final LocalDateTime _logTime; - public LogEntryImpl(long logId, long runId, String message, String stackTrace, Timestamp logTime) { + public LogEntryImpl(long logId, long runId, String message, String stackTrace, Instant logTime) { _logId = logId; _runId = runId; _message = message; _stackTrace = stackTrace; - _logTime = logTime.toLocalDateTime(); + _logTime = logTime.atZone(ZoneId.systemDefault()).toLocalDateTime(); } @Override diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRunner.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRunner.java index cf088fc..1abd466 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRunner.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/ScheduledTaskRunner.java @@ -36,6 +36,7 @@ import org.slf4j.MDC; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.ScheduleRunContext; import com.storebrand.scheduledtask.ScheduledTaskRegistry.ScheduleRunnable; @@ -80,7 +81,7 @@ class ScheduledTaskRunner implements ScheduledTask { private volatile boolean _active = true; private volatile boolean _runFlag = true; private volatile boolean _isRunning = false; - private volatile boolean _runOnce = false; + private volatile Optional _runOnce; private volatile boolean _justStarted = true; ScheduledTaskRunner(ScheduledTaskConfig config, ScheduleRunnable runnable, @@ -134,11 +135,12 @@ void runner() { updateStateFromSchedule(); _justStarted = false; } - if (_runOnce) { + if (_runOnce.isPresent()) { // We should run once, so we should not sleep but instead run the schedule now. log.info("Thread for Task '" + getName() + "' with nodeName '" + Host.getLocalHostName() + "' " - + " is master and set to run once (NOW) and then continue as set in " + + " is master and set to run once (NOW) by '" + _runOnce.get() + + "' and then continue as set in " + "schedule '" + getActiveCronExpressionInternal().toString() + "'."); } // E-> Have we passed the next run timestamp? @@ -223,13 +225,14 @@ else if (Instant.now(_clock).isBefore(_nextRun)) { updateStateFromSchedule(); // ?: Check if we should run now once regardless of when the schedule should actually run - if (_runOnce) { + if (_runOnce.isPresent()) { // -> Yes, we should only run once and then continue on the normal schedule plans. - _scheduledTaskRepository.setRunOnce(getName(), false); log.info("Thread for Task '" + getName() + "' with nodeName '" + Host.getLocalHostName() + "' " - + " is set to run once (NOW) and then continue as set in " + + " is set to run once (NOW) by '" + _runOnce.get() + + "' and then continue as set in " + "schedule '" + getActiveCronExpressionInternal().toString() + "'."); + _scheduledTaskRepository.setRunOnce(getName(), null); break SLEEP_LOOP; } @@ -324,7 +327,7 @@ private Optional updateStateFromSchedule() { _active = scheduleFromDb.get().isActive(); _nextRun = scheduleFromDb.get().getNextRun(); - _runOnce = scheduleFromDb.get().isRunOnce(); + _runOnce = scheduleFromDb.get().getRunOnce(); return scheduleFromDb; } @@ -354,12 +357,12 @@ private void runTask() { + "' is beginning to do the run according " + "to the set schedule '" + getActiveCronExpressionInternal().toString() + "'. Setting next run to '" + nextRun + "'."); - // ?: Is this schedule manually triggered? Ie set to run once. - if (ScheduledTaskRunner.this._runOnce) { - // -> Yes, this where set to run once and is manually triggered. so add a log line. + // ?: Is this schedule manually or programmatically triggered? Ie set to run once. + if (ScheduledTaskRunner.this._runOnce.isPresent()) { + // -> Yes, this where set to run once and is manually/programmatically triggered. so add a log line. // note, this is named runNow in the gui but runOnce in the db so we use the term used // in gui for the logging. - ctx.log("Manually started"); + ctx.log(_runOnce.get().name() + " started"); } // :: Try to run the code that the user wants, log if it fails. @@ -693,9 +696,16 @@ Instant nextScheduledRun(CronExpression cronExpression, Instant instant) { /** * Manual trigger the schedule to run now. + * + * Replaced with {@link #runNow(RunOnce)} */ @Override public void runNow() { + runNow(RunOnce.PROGRAMMATIC); + } + + @Override + public void runNow(RunOnce runOnce) { // ?: Are we in special test mode? if (_testMode) { // -> Yes, then we just run the task, and return. @@ -706,13 +716,12 @@ public void runNow() { } // Write to db we should run this schedule now regardless of the cronExpression - _scheduledTaskRepository.setRunOnce(getName(), true); + _scheduledTaskRepository.setRunOnce(getName(), runOnce); // Then wake the tread so it is aware that we should run now once synchronized (_syncObject) { _syncObject.notifyAll(); } } - // ===== Internal helper classes =================================================================================== /** diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryMasterLockRepository.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryMasterLockRepository.java index 924e99a..3ff06ee 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryMasterLockRepository.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryMasterLockRepository.java @@ -79,7 +79,7 @@ public boolean tryAcquireLock(String lockName, String nodeName) { return false; } Instant now = _clock.instant(); - // We should only allow to acquire the lock if the last_updated_time is older than 10 minutes. + // We should only allow acquiring the lock if the last_updated_time_utc is older than 10 minutes. // Then it means it is up for grabs. Instant lockShouldBeOlderThan = now.minus(10, ChronoUnit.MINUTES); diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepository.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepository.java index 5eb4a07..18a14e1 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepository.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepository.java @@ -36,6 +36,7 @@ import com.storebrand.scheduledtask.ScheduledTask.RetentionPolicy; import com.storebrand.scheduledtask.ScheduledTaskConfig; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.State; import com.storebrand.scheduledtask.SpringCronUtils.CronExpression; @@ -82,7 +83,7 @@ public int createSchedule(ScheduledTaskConfig config) { Instant nextRunInstant = nextRunTime.atZone(ZoneId.systemDefault()).toInstant(); _scheduledTaskDefinitions.put(config.getName(), config); - InMemorySchedule schedule = new InMemorySchedule(config.getName(), null, true, false, + InMemorySchedule schedule = new InMemorySchedule(config.getName(), null, true, null, nextRunInstant, _clock.instant()); _schedules.put(config.getName(), schedule); return 1; @@ -133,7 +134,7 @@ public int setActive(String scheduleName, boolean active) { } @Override - public int setRunOnce(String scheduleName, boolean runOnce) { + public int setRunOnce(String scheduleName, RunOnce runOnce) { InMemorySchedule schedule = _schedules.get(scheduleName); if (schedule == null) { return 0; @@ -333,11 +334,11 @@ private static final class InMemorySchedule implements Schedule { private final String _name; private final String _cronExpression; private volatile boolean _active; - private volatile boolean _runOnce; + private volatile RunOnce _runOnce; private volatile Instant _nextRun; private volatile Instant _lastUpdated; - private InMemorySchedule(String name, String cronExpression, boolean active, boolean runOnce, + private InMemorySchedule(String name, String cronExpression, boolean active, RunOnce runOnce, Instant nextRun, Instant lastUpdated) { _name = name; _cronExpression = cronExpression; @@ -359,7 +360,12 @@ public boolean isActive() { @Override public boolean isRunOnce() { - return _runOnce; + return getRunOnce().isPresent(); + } + + @Override + public Optional getRunOnce() { + return Optional.ofNullable(_runOnce); } @Override @@ -382,7 +388,7 @@ InMemorySchedule active(boolean active) { _lastUpdated); } - InMemorySchedule runOnce(boolean runOnce) { + InMemorySchedule runOnce(RunOnce runOnce) { return new InMemorySchedule(_name, _cronExpression, _active, runOnce, _nextRun, _lastUpdated); } diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/MasterLockRepository.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/MasterLockRepository.java index 8c0b303..ba8d6ac 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/MasterLockRepository.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/MasterLockRepository.java @@ -95,7 +95,7 @@ public interface MasterLockRepository { boolean releaseLock(String lockName, String nodeName); /** - * Used for the running host to keep the lock for 5 more minutes. If the lock_last_updated_time is updated + * Used for the running host to keep the lock for 5 more minutes. If the lock_last_updated_time_utc is updated * that means this host still has this master lock for another 5 minutes. After 5 minutes it means no-one has it * until 10 minutes has passed. At that time it is up for grabs again. * diff --git a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/ScheduledTaskRepository.java b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/ScheduledTaskRepository.java index aeaa819..a44d5aa 100644 --- a/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/ScheduledTaskRepository.java +++ b/scheduledtask-core/src/main/java/com/storebrand/scheduledtask/db/ScheduledTaskRepository.java @@ -23,9 +23,10 @@ import java.util.Map; import java.util.Optional; +import com.storebrand.scheduledtask.ScheduledTask.RetentionPolicy; import com.storebrand.scheduledtask.ScheduledTaskConfig; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; -import com.storebrand.scheduledtask.ScheduledTask.RetentionPolicy; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.State; import com.storebrand.scheduledtask.ScheduledTaskRegistryImpl; @@ -80,7 +81,7 @@ public interface ScheduledTaskRepository { */ int setActive(String scheduleName, boolean active); - int setRunOnce(String scheduleName, boolean runOnce); + int setRunOnce(String scheduleName, RunOnce runOnce); /** * Get all schedules in the database diff --git a/scheduledtask-core/src/test/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepositoryTest.java b/scheduledtask-core/src/test/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepositoryTest.java index ff7ec54..261ced0 100644 --- a/scheduledtask-core/src/test/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepositoryTest.java +++ b/scheduledtask-core/src/test/java/com/storebrand/scheduledtask/db/InMemoryScheduledTaskRepositoryTest.java @@ -37,6 +37,7 @@ import com.storebrand.scheduledtask.ScheduledTaskConfig; import com.storebrand.scheduledtask.ScheduledTaskRegistry; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.db.InMemoryMasterLockRepositoryTest.ClockMock; import com.storebrand.scheduledtask.db.ScheduledTaskRepository.ScheduledRunDto; @@ -279,7 +280,7 @@ public void setRunOnce() { LocalDateTime updateTime = LocalDateTime.of(2021, 3, 3, 12, 12); _clock.setFixedClock(updateTime); Optional beforeSettingRunOnce = schedulerRep.getSchedule("test-schedule"); - schedulerRep.setRunOnce("test-schedule", true); + schedulerRep.setRunOnce("test-schedule", RunOnce.PROGRAMMATIC); // :: Assert assertTrue(beforeSettingRunOnce.isPresent()); @@ -294,6 +295,8 @@ public void setRunOnce() { // No cron expression should be set since we are not overriding the cron expression assertNull(afterSetRunOnce.get().getOverriddenCronExpression().orElse(null)); assertEquals(insertTimeInstant, afterSetRunOnce.get().getLastUpdated()); + assertFalse(beforeSettingRunOnce.get().getRunOnce().isPresent()); + assertEquals(RunOnce.PROGRAMMATIC, afterSetRunOnce.get().getRunOnce().orElse(null)); } @Test diff --git a/scheduledtask-db-sql/build.gradle b/scheduledtask-db-sql/build.gradle index 69c27d5..f56414a 100644 --- a/scheduledtask-db-sql/build.gradle +++ b/scheduledtask-db-sql/build.gradle @@ -33,6 +33,7 @@ dependencies { // Placing logback-classic on testCompile path as we need some form of logging implementation. //testImplementation "org.slf4j:slf4j-api:$slf4jVersion" + testImplementation "org.slf4j:slf4j-api:$slf4jVersion" testRuntimeOnly "ch.qos.logback:logback-classic:$logbackClassicVersion" } diff --git a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/MasterLockSqlRepository.java b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/MasterLockSqlRepository.java index c9e52de..9cf0715 100644 --- a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/MasterLockSqlRepository.java +++ b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/MasterLockSqlRepository.java @@ -26,9 +26,11 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Calendar; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.TimeZone; import javax.sql.DataSource; @@ -37,26 +39,30 @@ import com.storebrand.scheduledtask.ScheduledTaskRegistry.MasterLock; import com.storebrand.scheduledtask.ScheduledTaskRegistryImpl; -import com.storebrand.scheduledtask.db.sql.TableInspector.TableValidationException; import com.storebrand.scheduledtask.db.MasterLockRepository; +import com.storebrand.scheduledtask.db.sql.TableInspector.TableValidationException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** - * Repository used by {@link ScheduledTaskRegistryImpl} to handle master election for a node. - * On each {@link #tryAcquireLock(String, String)} it will also try to insert the lock, if the node managed to insert it - * then that node has the lock. + * Repository used by {@link ScheduledTaskRegistryImpl} to handle master election for a node. On each + * {@link #tryAcquireLock(String, String)} it will also try to insert the lock if the node managed to insert it then + * that node has the lock. *

- * After a lock has been acquired for a node it has to do the {@link #keepLock(String, String)} within the next 5 min - * in order to be allowed to keep it. If it does not update withing that timespan it has to wait until the + * After a lock has been acquired for a node it has to do the {@link #keepLock(String, String)} within the next 5 min in + * order to be allowed to keep it. If it does not update withing that timespan, it has to wait until the * {@link MasterLockDto#getLockLastUpdatedTime()} is over 10 min old before any node can acquire it again. This means - * there is a 5 min gap where no node can aquire the lock at all. + * there is a 5 min gap where no node can acquire the lock at all. * * @author Dag Bertelsen - dag.lennart.bertelsen@storebrand.no - dabe@dagbertelsen.com - 2021.03 */ public class MasterLockSqlRepository implements MasterLockRepository { private static final Logger log = LoggerFactory.getLogger(MasterLockSqlRepository.class); public static final String MASTER_LOCK_TABLE = "stb_schedule_master_locker"; + // The UTC timezone is used to make sure we are not affected by daylight-saving time, IE this is a fixed timezone + // for when we are storing the time in the database. The Timestamp.from(Instant) will convert the Instant to the + // system default timezone if none are specified. + private static final TimeZone TIME_ZONE_UTC = TimeZone.getTimeZone("UTC"); private final DataSource _dataSource; private final Clock _clock; @@ -86,22 +92,22 @@ public boolean tryCreateMissingLock(String lockName) { public boolean tryAcquireLock(String lockName, String nodeName) { // This lock already should exist so try to acquire it. String sql = "UPDATE " + MASTER_LOCK_TABLE - + " SET node_name = ?, lock_taken_time = ?, lock_last_updated_time = ? " + + " SET node_name = ?, lock_taken_time_utc = ?, lock_last_updated_time_utc = ? " + " WHERE lock_name = ? " // (lockLastUpdated <= $now - 10 minutes)). Can only acquire lock if the lastUpdated is more than 10 min old - + " AND lock_last_updated_time <= ?"; + + " AND lock_last_updated_time_utc <= ?"; try (Connection sqlConnection = _dataSource.getConnection(); - PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { + PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { Instant now = _clock.instant(); - // We should only allow to acquire the lock if the last_updated_time is older than 10 minutes. + // We should only allow acquiring the lock if the last_updated_time_utc is older than 10 minutes. // Then it means it is up for grabs. Instant lockShouldBeOlderThan = now.minus(10, ChronoUnit.MINUTES); pStmt.setString(1, nodeName); - pStmt.setTimestamp(2, Timestamp.from(now)); - pStmt.setTimestamp(3, Timestamp.from(now)); + pStmt.setTimestamp(2, Timestamp.from(now), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(3, Timestamp.from(now), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(4, lockName); - pStmt.setTimestamp(5, Timestamp.from(lockShouldBeOlderThan)); + pStmt.setTimestamp(5, Timestamp.from(lockShouldBeOlderThan), Calendar.getInstance(TIME_ZONE_UTC)); return pStmt.executeUpdate() == 1; } catch (SQLException e) { @@ -113,14 +119,14 @@ public boolean tryAcquireLock(String lockName, String nodeName) { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public boolean releaseLock(String lockName, String nodeName) { String sql = "UPDATE " + MASTER_LOCK_TABLE - + " SET lock_taken_time = ?, lock_last_updated_time = ? " + + " SET lock_taken_time_utc = ?, lock_last_updated_time_utc = ? " + " WHERE lock_name = ? " + " AND node_name = ?"; try (Connection sqlConnection = _dataSource.getConnection(); - PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - pStmt.setTimestamp(1, Timestamp.from(Instant.EPOCH)); - pStmt.setTimestamp(2, Timestamp.from(Instant.EPOCH)); + PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { + pStmt.setTimestamp(1, Timestamp.from(Instant.EPOCH), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(2, Timestamp.from(Instant.EPOCH), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, lockName); pStmt.setString(4, nodeName); return pStmt.executeUpdate() == 1; @@ -134,21 +140,21 @@ public boolean releaseLock(String lockName, String nodeName) { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public boolean keepLock(String lockName, String nodeName) { String sql = "UPDATE " + MASTER_LOCK_TABLE - + " SET node_name = ?,lock_last_updated_time = ? " + + " SET node_name = ?,lock_last_updated_time_utc = ? " + " WHERE lock_name = ? " + " AND node_name = ? " // (lockLastUpdated >= $now - 5 minutes)). Can only do keeplock within 5 min after it was last updated. - + " AND lock_last_updated_time >= ?"; + + " AND lock_last_updated_time_utc >= ?"; try (Connection sqlConnection = _dataSource.getConnection(); - PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { + PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { Instant now = _clock.instant(); Instant lockShouldBeNewerThan = now.minus(5, ChronoUnit.MINUTES); pStmt.setString(1, nodeName); - pStmt.setTimestamp(2, Timestamp.from(now)); + pStmt.setTimestamp(2, Timestamp.from(now), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, lockName); pStmt.setString(4, nodeName); - pStmt.setTimestamp(5, Timestamp.from(lockShouldBeNewerThan)); + pStmt.setTimestamp(5, Timestamp.from(lockShouldBeNewerThan), Calendar.getInstance(TIME_ZONE_UTC)); return pStmt.executeUpdate() == 1; } catch (SQLException e) { @@ -162,7 +168,7 @@ public List getLocks() throws SQLException { String sql = "SELECT * FROM " + MASTER_LOCK_TABLE; try (Connection sqlConnection = _dataSource.getConnection(); - PreparedStatement pStmt = sqlConnection.prepareStatement(sql); + PreparedStatement pStmt = sqlConnection.prepareStatement(sql); ResultSet result = pStmt.executeQuery()) { List masterLocks = new ArrayList<>(); @@ -170,12 +176,12 @@ public List getLocks() throws SQLException { MasterLockDto row = new MasterLockDto( result.getString("lock_name"), result.getString("node_name"), - result.getTimestamp("lock_taken_time"), - result.getTimestamp("lock_last_updated_time")); + result.getTimestamp("lock_taken_time_utc", Calendar.getInstance(TIME_ZONE_UTC)), + result.getTimestamp("lock_last_updated_time_utc", Calendar.getInstance(TIME_ZONE_UTC))); masterLocks.add(row); } return Collections.unmodifiableList(masterLocks); - } + } } @Override @@ -193,15 +199,15 @@ public Optional getLock(String lockName) { MasterLockDto dbo = new MasterLockDto( result.getString("lock_name"), result.getString("node_name"), - result.getTimestamp("lock_taken_time"), - result.getTimestamp("lock_last_updated_time")); + result.getTimestamp("lock_taken_time_utc", Calendar.getInstance(TIME_ZONE_UTC)), + result.getTimestamp("lock_last_updated_time_utc", Calendar.getInstance(TIME_ZONE_UTC))); return Optional.of(dbo); } // E-> No, we did not find anything return Optional.empty(); } - } + } catch (SQLException throwables) { throw new RuntimeException(throwables); } @@ -210,7 +216,7 @@ public Optional getLock(String lockName) { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") private boolean tryCreateLockInternal(String lockName, String nodeName, Instant time) { String sql = "INSERT INTO " + MASTER_LOCK_TABLE - + " (lock_name, node_name, lock_taken_time, lock_last_updated_time) " + + " (lock_name, node_name, lock_taken_time_utc, lock_last_updated_time_utc) " + " SELECT ?, ?, ?, ? " + " WHERE NOT EXISTS (SELECT lock_name FROM " + MASTER_LOCK_TABLE + " WHERE lock_name = ?)"; @@ -221,8 +227,8 @@ private boolean tryCreateLockInternal(String lockName, String nodeName, Instant PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { pStmt.setString(1, lockName); pStmt.setString(2, nodeName); - pStmt.setTimestamp(3, Timestamp.from(time)); - pStmt.setTimestamp(4, Timestamp.from(time)); + pStmt.setTimestamp(3, Timestamp.from(time), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(4, Timestamp.from(time), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(5, lockName); return pStmt.executeUpdate() == 1; } @@ -237,10 +243,21 @@ private void validateTableVersion() { // Get the tableVersion int version = inspector.getTableVersion(); if (version != TableInspector.VALID_VERSION) { + String message = TableInspector.TABLE_VERSION + ".version has the version '" + version + "' " + + "while we expected '" + TableInspector.VALID_VERSION + "'. "; + + // ?: Are we upgrading from the initial version? + if (version == 1) { + // -> Yes, we are upgrading from version 1 to 2 + throw new TableValidationException(message + + "Seems you are using version 1 of the required tables, you must upgrade to version 2.\n" + + " An example SQL script to do this is located at:\n" + + inspector.getMigrationFileLocation(TableInspector.MIGRATE_FROM_V1_TO_V2_SQL)); + + } + // NO-> different version than what we expected, this means these tables may not be correct. - log.error(TableInspector.TABLE_VERSION + " has the version '" + version + "' " - + "while we expected '" + TableInspector.VALID_VERSION + "'. " - + inspector.getMigrationLocationMessage()); + log.error(message + inspector.getMigrationLocationMessage()); } } @@ -250,7 +267,7 @@ private void validateTableStructure() { if (inspector.amountOfColumns() == 0) { // Table was not found throw new TableValidationException("Table '" + MASTER_LOCK_TABLE + "' where not found, " - + "create the tables by manually importing '" + inspector.getMigrationFileLocation() + "'"); + + "create the tables by manually importing '" + inspector.getInitialCreationFileLocation() + "'"); } // :: Verify we have all the table columns and their sizes @@ -260,10 +277,10 @@ private void validateTableStructure() { inspector.validateColumn("node_name", 255, false, JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); - inspector.validateColumn("lock_taken_time", false, + inspector.validateColumn("lock_taken_time_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); - inspector.validateColumn("lock_last_updated_time", false, + inspector.validateColumn("lock_last_updated_time_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); } diff --git a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepository.java b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepository.java index 61f8c41..e9c972b 100644 --- a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepository.java +++ b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepository.java @@ -30,11 +30,13 @@ import java.time.LocalDateTime; import java.time.ZoneId; import java.util.ArrayList; +import java.util.Calendar; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.TimeZone; import java.util.concurrent.ConcurrentHashMap; import javax.sql.DataSource; @@ -42,18 +44,19 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.storebrand.scheduledtask.ScheduledTaskConfig; -import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; import com.storebrand.scheduledtask.ScheduledTask.RetentionPolicy; +import com.storebrand.scheduledtask.ScheduledTaskConfig; import com.storebrand.scheduledtask.ScheduledTaskRegistry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.State; import com.storebrand.scheduledtask.ScheduledTaskRegistryImpl; import com.storebrand.scheduledtask.ScheduledTaskRegistryImpl.LogEntryImpl; import com.storebrand.scheduledtask.ScheduledTaskRegistryImpl.ScheduleImpl; import com.storebrand.scheduledtask.SpringCronUtils.CronExpression; -import com.storebrand.scheduledtask.db.sql.TableInspector.TableValidationException; import com.storebrand.scheduledtask.db.ScheduledTaskRepository; +import com.storebrand.scheduledtask.db.sql.TableInspector.TableValidationException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -68,6 +71,10 @@ public class ScheduledTaskSqlRepository implements ScheduledTaskRepository { public static final String SCHEDULE_TASK_TABLE = "stb_schedule"; public static final String SCHEDULE_RUN_TABLE = "stb_schedule_run"; public static final String SCHEDULE_LOG_ENTRY_TABLE = "stb_schedule_log_entry"; + // The UTC timezone is used to make sure we are not affected by daylight-saving time, IE this is a fixed timezone + // for when we are storing the time in the database. The Timestamp.from(Instant) will convert the Instant to the + // system default timezone if none are specified. + private static final TimeZone TIME_ZONE_UTC = TimeZone.getTimeZone("UTC"); private final DataSource _dataSource; private final Clock _clock; private ConcurrentHashMap _scheduledTaskDefinitions = new ConcurrentHashMap<>(); @@ -88,7 +95,7 @@ public int createSchedule(ScheduledTaskConfig config) { _scheduledTaskDefinitions.put(config.getName(), config); String sql = "INSERT INTO " + SCHEDULE_TASK_TABLE - + " (schedule_name, is_active, run_once, next_run, last_updated) " + + " (schedule_name, is_active, run_once, next_run_utc, last_updated_utc) " + " SELECT ?, ?, ?, ?, ?" + " WHERE NOT EXISTS (SELECT schedule_name FROM " + SCHEDULE_TASK_TABLE + " WHERE schedule_name = ?)"; @@ -105,9 +112,9 @@ public int createSchedule(ScheduledTaskConfig config) { // All schedules when created is by default active pStmt.setBoolean(2, true); // All new schedules should not have run once set - pStmt.setBoolean(3, false); - pStmt.setTimestamp(4, Timestamp.from(nextRunInstant)); - pStmt.setTimestamp(5, Timestamp.from(_clock.instant())); + pStmt.setString(3, null); + pStmt.setTimestamp(4, Timestamp.from(nextRunInstant), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(5, Timestamp.from(_clock.instant()), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(6, config.getName()); int ret = pStmt.executeUpdate(); if (ret == 1) { @@ -128,7 +135,7 @@ public int createSchedule(ScheduledTaskConfig config) { @SuppressFBWarnings({"RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"}) public int setTaskOverridenCron(String scheduleName, String overrideCronExpression) { String sql = "UPDATE " + SCHEDULE_TASK_TABLE - + " SET cron_expression = ?, last_updated = ? " + + " SET cron_expression = ?, last_updated_utc = ? " + " WHERE schedule_name = ?"; log.info("Setting override cronExpression for [" + scheduleName + "] to [" + overrideCronExpression + "]"); @@ -136,7 +143,7 @@ public int setTaskOverridenCron(String scheduleName, String overrideCronExpressi try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { pStmt.setString(1, overrideCronExpression); - pStmt.setTimestamp(2, Timestamp.from(_clock.instant())); + pStmt.setTimestamp(2, Timestamp.from(_clock.instant()), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, scheduleName); int ret = pStmt.executeUpdate(); updateNextRun(scheduleName); @@ -151,7 +158,7 @@ public int setTaskOverridenCron(String scheduleName, String overrideCronExpressi @SuppressFBWarnings({"RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"}) public int updateNextRun(String scheduleName) { String sql = "UPDATE " + SCHEDULE_TASK_TABLE - + " SET next_run = ?, last_updated = ? " + + " SET next_run_utc = ?, last_updated_utc = ? " + " WHERE schedule_name = ?"; Schedule schedule = getSchedule(scheduleName).orElseThrow(); @@ -166,8 +173,8 @@ public int updateNextRun(String scheduleName) { try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - pStmt.setTimestamp(1, Timestamp.from(nextRunInstant)); - pStmt.setTimestamp(2, Timestamp.from(_clock.instant())); + pStmt.setTimestamp(1, Timestamp.from(nextRunInstant), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(2, Timestamp.from(_clock.instant()), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, scheduleName); return pStmt.executeUpdate(); } @@ -198,7 +205,7 @@ public int setActive(String scheduleName, boolean active) { @Override @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") - public int setRunOnce(String scheduleName, boolean runOnce) { + public int setRunOnce(String scheduleName, RunOnce runOnce) { String sql = "UPDATE " + SCHEDULE_TASK_TABLE + " SET run_once = ? " + " WHERE schedule_name = ?"; @@ -207,7 +214,7 @@ public int setRunOnce(String scheduleName, boolean runOnce) { try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - pStmt.setBoolean(1, runOnce); + pStmt.setString(1, runOnce != null ? runOnce.name() : null); pStmt.setString(2, scheduleName); return pStmt.executeUpdate(); } @@ -230,10 +237,10 @@ public Map getSchedules() { ScheduleImpl row = new ScheduleImpl( result.getString("schedule_name"), result.getBoolean("is_active"), - result.getBoolean("run_once"), + RunOnce.fromString(result.getString("run_once")), result.getString("cron_expression"), - result.getTimestamp("next_run").toInstant(), - result.getTimestamp("last_updated").toInstant()); + result.getTimestamp("next_run_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant(), + result.getTimestamp("last_updated_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant()); schedules.add(row); } return schedules.stream() @@ -260,10 +267,10 @@ public Optional getSchedule(String scheduleName) { ScheduleImpl schedule = new ScheduleImpl( result.getString("schedule_name"), result.getBoolean("is_active"), - result.getBoolean("run_once"), + RunOnce.fromString(result.getString("run_once")), result.getString("cron_expression"), - result.getTimestamp("next_run").toInstant(), - result.getTimestamp("last_updated").toInstant()); + result.getTimestamp("next_run_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant(), + result.getTimestamp("last_updated_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant()); return Optional.of(schedule); } @@ -280,7 +287,7 @@ public Optional getSchedule(String scheduleName) { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public long addScheduleRun(String scheduleName, String hostname, Instant runStart, String statusMsg) { String sql = "INSERT INTO " + SCHEDULE_RUN_TABLE - + " (schedule_name, hostname, status, status_msg, run_start, status_time) " + + " (schedule_name, hostname, status, status_msg, run_start_utc, status_time_utc) " + " VALUES (?, ?, ?, ?, ?, ?)"; log.debug("Adding scheduleRun for scheuleName [" + scheduleName + "]"); @@ -291,8 +298,8 @@ public long addScheduleRun(String scheduleName, String hostname, Instant runStar pStmt.setString(2, hostname); pStmt.setString(3, ScheduledTaskRegistry.State.STARTED.toString()); pStmt.setString(4, statusMsg); - pStmt.setTimestamp(5, Timestamp.from(runStart)); - pStmt.setTimestamp(6, Timestamp.from(_clock.instant())); + pStmt.setTimestamp(5, Timestamp.from(runStart), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(6, Timestamp.from(_clock.instant()), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.execute(); try (ResultSet rs = pStmt.getGeneratedKeys()) { if (rs.next()) { @@ -312,7 +319,7 @@ public long addScheduleRun(String scheduleName, String hostname, Instant runStar public boolean setStatus(long runId, State state, String statusMsg, String statusStackTrace, Instant statusTime) { String sql = "UPDATE " + SCHEDULE_RUN_TABLE - + " SET status = ?, status_msg = ?, status_stacktrace = ?, status_time = ? " + + " SET status = ?, status_msg = ?, status_stacktrace = ?, status_time_utc = ? " + " WHERE run_id = ?"; if (state.equals(ScheduledTaskRegistry.State.STARTED)) { @@ -322,12 +329,12 @@ public boolean setStatus(long runId, State state, String statusMsg, String statu try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - // We should only allow to acquire the lock if the last_updated_time is older than 10 minutes. + // We should only allow to acquire the lock if the last_updated_time_utc is older than 10 minutes. // Then it means it is up for grabs. pStmt.setString(1, state.toString()); pStmt.setString(2, statusMsg); pStmt.setString(3, statusStackTrace); - pStmt.setTimestamp(4, Timestamp.from(statusTime)); + pStmt.setTimestamp(4, Timestamp.from(statusTime), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setLong(5, runId); return pStmt.executeUpdate() == 1; } @@ -373,7 +380,7 @@ public Optional getScheduledRun(long runId) { public Optional getLastRunForSchedule(String scheduleName) { String sql = "SELECT TOP(1) * FROM " + SCHEDULE_RUN_TABLE + " WHERE schedule_name = ? " - + " ORDER BY run_start DESC"; + + " ORDER BY run_start_utc DESC"; try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { @@ -399,12 +406,12 @@ public Optional getLastRunForSchedule(String scheduleName) { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public List getLastScheduleRuns() { String sql = " SELECT * " - + " FROM (SELECT SCHEDULE_NAME, max(run_start) as run_start " + + " FROM (SELECT SCHEDULE_NAME, max(run_start_utc) as run_start_utc " + " from " + SCHEDULE_RUN_TABLE + " group by SCHEDULE_NAME) AS lr " + " INNER JOIN " + SCHEDULE_RUN_TABLE + " as sr " - + " ON sr.SCHEDULE_NAME = lr.SCHEDULE_NAME AND sr.run_start = lr.run_start" - + " ORDER BY run_start DESC "; + + " ON sr.SCHEDULE_NAME = lr.SCHEDULE_NAME AND sr.run_start_utc = lr.run_start_utc" + + " ORDER BY run_start_utc DESC "; try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql); @@ -428,15 +435,15 @@ public List getLastScheduleRuns() { @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public List getScheduleRunsBetween(String scheduleName, LocalDateTime from, LocalDateTime to) { String sql = "SELECT * FROM " + SCHEDULE_RUN_TABLE - + " WHERE run_start >= ? " - + " AND run_start <= ? " + + " WHERE run_start_utc >= ? " + + " AND run_start_utc <= ? " + " AND schedule_name = ? " - + " ORDER BY run_start DESC"; + + " ORDER BY run_start_utc DESC"; try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - pStmt.setTimestamp(1, Timestamp.valueOf(from)); - pStmt.setTimestamp(2, Timestamp.valueOf(to)); + pStmt.setTimestamp(1, Timestamp.valueOf(from), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(2, Timestamp.valueOf(to), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, scheduleName); try (ResultSet result = pStmt.executeQuery()) { @@ -459,18 +466,18 @@ public List getScheduleRunsBetween(String scheduleName, LocalDa @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public Map> getLogEntriesByRunId(String scheduleName, LocalDateTime from, LocalDateTime to) { String runIDsForScheduleName = "SELECT run_id FROM " + SCHEDULE_RUN_TABLE - + " WHERE run_start >= ? " - + " AND run_start <= ? " + + " WHERE run_start_utc >= ? " + + " AND run_start_utc <= ? " + " AND schedule_name = ?"; String sql = "SELECT * FROM " + SCHEDULE_LOG_ENTRY_TABLE + " WHERE run_id IN (" + runIDsForScheduleName + ")" - + " ORDER BY log_time DESC"; + + " ORDER BY log_time_utc DESC"; try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { - pStmt.setTimestamp(1, Timestamp.valueOf(from)); - pStmt.setTimestamp(2, Timestamp.valueOf(to)); + pStmt.setTimestamp(1, Timestamp.valueOf(from), Calendar.getInstance(TIME_ZONE_UTC)); + pStmt.setTimestamp(2, Timestamp.valueOf(to), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.setString(3, scheduleName); Map> logEntriesByRunId = new HashMap<>(); @@ -483,7 +490,8 @@ public Map> getLogEntriesByRunId(String scheduleName, Local result.getLong("run_id"), result.getString("log_msg"), result.getString("log_stacktrace"), - result.getTimestamp("log_time")); + result.getTimestamp("log_time_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant() + ); logEntriesByRunId.computeIfAbsent(runId, notUsed -> new ArrayList<>()).add(logEntry); } @@ -499,7 +507,7 @@ public Map> getLogEntriesByRunId(String scheduleName, Local @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public void addLogEntry(long runId, LocalDateTime logTime, String message, String stackTrace) { String sql = "INSERT INTO " + SCHEDULE_LOG_ENTRY_TABLE - + " (run_id, log_msg, log_stacktrace, log_time) " + + " (run_id, log_msg, log_stacktrace, log_time_utc) " + " VALUES (?, ?, ?, ?)"; log.debug("Adding logEntry for runId [" + runId + "], " @@ -511,7 +519,7 @@ public void addLogEntry(long runId, LocalDateTime logTime, String message, Strin pStmt.setLong(1, runId); pStmt.setString(2, message); pStmt.setString(3, stackTrace); - pStmt.setTimestamp(4, Timestamp.valueOf(logTime)); + pStmt.setTimestamp(4, Timestamp.valueOf(logTime), Calendar.getInstance(TIME_ZONE_UTC)); pStmt.executeUpdate(); } catch (SQLException e) { @@ -524,7 +532,7 @@ public void addLogEntry(long runId, LocalDateTime logTime, String message, Strin public List getLogEntries(long runId) { String sql = "SELECT * FROM " + SCHEDULE_LOG_ENTRY_TABLE + " WHERE run_id = ? " - + " ORDER BY log_time ASC "; + + " ORDER BY log_time_utc ASC "; try (Connection sqlConnection = _dataSource.getConnection(); PreparedStatement pStmt = sqlConnection.prepareStatement(sql)) { pStmt.setLong(1, runId); @@ -538,7 +546,8 @@ public List getLogEntries(long runId) { result.getLong("run_id"), result.getString("log_msg"), result.getString("log_stacktrace"), - result.getTimestamp("log_time")); + result.getTimestamp("log_time_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant() + ); logEntries.add(logEntry); } @@ -559,8 +568,8 @@ private ScheduledRunDto fromResultSet(ResultSet result) throws SQLException { state, result.getString("status_msg"), result.getString("status_stacktrace"), - result.getTimestamp("run_start").toInstant(), - result.getTimestamp("status_time").toInstant()); + result.getTimestamp("run_start_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant(), + result.getTimestamp("status_time_utc", Calendar.getInstance(TIME_ZONE_UTC)).toInstant()); } // ===== Retention policy ================================================================= @@ -678,7 +687,7 @@ public void executeRetentionPolicy(String scheduleName, RetentionPolicy retentio private Optional findDeleteOlderForKeepMax(Connection sqlConnection, String scheduleName, int keep, State status) throws SQLException { - String sql = "SELECT run_start FROM " + SCHEDULE_RUN_TABLE + " WHERE schedule_name = ? "; + String sql = "SELECT run_start_utc FROM " + SCHEDULE_RUN_TABLE + " WHERE schedule_name = ? "; // ?: Are we querying for a specific status? if (status != null) { // Yes -> Add specific status to query @@ -688,7 +697,7 @@ private Optional findDeleteOlderForKeepMax(Connection sqlConnecti // No -> Then we should delete DONE, NOOP and FAILED statuses. We should not delete runs that are not complete. sql += " AND status IN (?, ?, ?) "; } - sql += " ORDER BY schedule_name, run_start DESC, status " + sql += " ORDER BY schedule_name, run_start_utc DESC, status " + " OFFSET ? ROWS " + " FETCH NEXT 1 ROWS ONLY "; @@ -709,8 +718,11 @@ private Optional findDeleteOlderForKeepMax(Connection sqlConnecti ResultSet rs = pStmt.executeQuery(); if (rs.next()) { - Timestamp runStart = rs.getTimestamp("run_start"); - return Optional.of(runStart.toLocalDateTime()); + Timestamp runStart = rs.getTimestamp("run_start_utc", Calendar.getInstance(TIME_ZONE_UTC)); + if (runStart == null) { + return Optional.empty(); + } + return Optional.of(runStart.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime()); } } return Optional.empty(); @@ -720,7 +732,7 @@ private int executeDelete(Connection sqlConnection, String scheduleName, LocalDateTime deleteOlder, State status) throws SQLException { String where = " WHERE schedule_name = ?" - + " AND run_start <= ?"; + + " AND run_start_utc<= ?"; // ?: Are we querying for a specific status? if (status != null) { // Yes -> Add specific status to query @@ -739,7 +751,7 @@ private int executeDelete(Connection sqlConnection, try (PreparedStatement pStmt = sqlConnection.prepareStatement(deleteLogs)) { pStmt.setString(1, scheduleName); - pStmt.setTimestamp(2, Timestamp.valueOf(deleteOlder)); + pStmt.setTimestamp(2, Timestamp.valueOf(deleteOlder), Calendar.getInstance(TIME_ZONE_UTC)); if (status != null) { pStmt.setString(3, status.name()); } @@ -754,7 +766,7 @@ private int executeDelete(Connection sqlConnection, try (PreparedStatement pStmt = sqlConnection.prepareStatement(deleteRuns)) { pStmt.setString(1, scheduleName); - pStmt.setTimestamp(2, Timestamp.valueOf(deleteOlder)); + pStmt.setTimestamp(2, Timestamp.valueOf(deleteOlder), Calendar.getInstance(TIME_ZONE_UTC)); if (status != null) { pStmt.setString(3, status.name()); } @@ -779,7 +791,7 @@ private void validateScheduledTaskTableStructure() { if (inspector.amountOfColumns() == 0) { // Table was not found throw new TableValidationException("Table '" + SCHEDULE_TASK_TABLE + "' where not found, " - + "create the tables by manually importing '" + inspector.getMigrationFileLocation() + "'"); + + "create the tables by manually importing '" + inspector.getInitialCreationFileLocation() + "'"); } // ----- We got the same version as we expected, but do a sanity check regardless. @@ -790,16 +802,16 @@ private void validateScheduledTaskTableStructure() { inspector.validateColumn("is_active", false, JDBCType.BOOLEAN, JDBCType.BIT, JDBCType.TINYINT, JDBCType.SMALLINT, JDBCType.INTEGER, JDBCType.NUMERIC); - inspector.validateColumn("run_once", false, - JDBCType.BOOLEAN, JDBCType.BIT, JDBCType.TINYINT, JDBCType.SMALLINT, JDBCType.INTEGER, JDBCType.NUMERIC); + inspector.validateColumn("run_once", 100, true, + JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); inspector.validateColumn("cron_expression", 255, true, JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); - inspector.validateColumn("next_run", false, + inspector.validateColumn("next_run_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); - inspector.validateColumn("last_updated", false, + inspector.validateColumn("last_updated_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); } @@ -812,7 +824,7 @@ private void validateScheduleRunTableStructure() { if (inspector.amountOfColumns() == 0) { // Table was not found throw new TableValidationException("Table '" + SCHEDULE_RUN_TABLE + "' where not found, " - + "create the tables by manually importing '" + inspector.getMigrationFileLocation() + "'"); + + "create the tables by manually importing '" + inspector.getInitialCreationFileLocation() + "'"); } // :: Verify we have all the table columns and their sizes @@ -825,7 +837,7 @@ private void validateScheduleRunTableStructure() { inspector.validateColumn("hostname", 255, false, JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); - inspector.validateColumn("run_start", false, + inspector.validateColumn("run_start_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); inspector.validateColumn("status", 250, true, @@ -841,7 +853,7 @@ private void validateScheduleRunTableStructure() { inspector.validateColumn("status_stacktrace", 3000, true, JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); - inspector.validateColumn("status_time", false, + inspector.validateColumn("status_time_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); } @@ -855,7 +867,7 @@ private void validateScheduleLogEntryTableStructure() { if (inspector.amountOfColumns() == 0) { // Table was not found throw new TableValidationException("Table '" + SCHEDULE_LOG_ENTRY_TABLE + "' where not found, " - + "create the tables by manually importing '" + inspector.getMigrationFileLocation() + "'"); + + "create the tables by manually importing '" + inspector.getInitialCreationFileLocation() + "'"); } // :: Verify we have all the table columns and their sizes @@ -875,7 +887,7 @@ private void validateScheduleLogEntryTableStructure() { inspector.validateColumn("log_stacktrace", 3000, true, JDBCType.VARCHAR, JDBCType.NVARCHAR, JDBCType.LONGVARCHAR, JDBCType.LONGNVARCHAR); - inspector.validateColumn("log_time", false, + inspector.validateColumn("log_time_utc", false, JDBCType.TIMESTAMP, JDBCType.TIME, JDBCType.TIME_WITH_TIMEZONE, JDBCType.TIMESTAMP_WITH_TIMEZONE); } diff --git a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/TableInspector.java b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/TableInspector.java index 1daa41b..2bfc376 100644 --- a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/TableInspector.java +++ b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/TableInspector.java @@ -48,12 +48,13 @@ class TableInspector { private static final Logger log = LoggerFactory.getLogger(TableInspector.class); public static final String TABLE_VERSION = "stb_schedule_table_version"; - public static final int VALID_VERSION = 1; + public static final int VALID_VERSION = 2; private final Map _tableColumns; private final Map _primaryKeys; private final Map _foreignKeys; private final String _tableName; - private static final String MIGRATION_FILE_NAME = "V_1__Create_initial_tables.sql"; + private static final String CREATE_INITIAL_TABLES_SQL = "V_2__Create_initial_tables.sql"; + public static final String MIGRATE_FROM_V1_TO_V2_SQL = "V_1_to_V_2_migrate_example.sql"; private final DataSource _dataSource; @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", @@ -74,7 +75,7 @@ public TableInspector(DataSource dataSource, String tableName) { } /** - * Checks the table version table to see if the database are using the correct {@link #MIGRATION_FILE_NAME} version. + * Checks the table version table to see if the database are using the correct {@link #CREATE_INITIAL_TABLES_SQL} version. * */ @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", @@ -206,14 +207,21 @@ public boolean hasForeignKey(String columnName, String keyOwnerTableName, String /** * Returns the location of the migrationfile containing the creation of the required tables. */ - public final String getMigrationFileLocation() { + public final String getInitialCreationFileLocation() { + return getMigrationFileLocation(CREATE_INITIAL_TABLES_SQL); + } + + /** + * Returns the location of the migrationfile containing the creation of the required tables. + */ + public final String getMigrationFileLocation(String filename) { URL codeSourceLocation = this.getClass().getProtectionDomain().getCodeSource().getLocation(); - return codeSourceLocation + "com/storebrand/scheduledtask/db/sql/" + MIGRATION_FILE_NAME; + return codeSourceLocation + "com/storebrand/scheduledtask/db/sql/" + filename; } public final String getMigrationLocationMessage() { return "Make sure you are using the migration script " - + "'" + getMigrationFileLocation() + "' to create the required tables"; + + "'" + getInitialCreationFileLocation() + "' to create the required tables"; } /** diff --git a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1_to_V_2_migrate_example.sql b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1_to_V_2_migrate_example.sql new file mode 100644 index 0000000..9d36148 --- /dev/null +++ b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1_to_V_2_migrate_example.sql @@ -0,0 +1,28 @@ +-- Rename datetime columns to have _utc suffix +ALTER TABLE stb_schedule_master_locker + RENAME COLUMN lock_taken_time TO lock_taken_time_utc; + +ALTER TABLE stb_schedule_master_locker + RENAME COLUMN lock_last_updated_time TO lock_last_updated_time_utc; + +ALTER TABLE stb_schedule + RENAME COLUMN next_run TO next_run_utc; + +ALTER TABLE stb_schedule + RENAME COLUMN last_updated TO last_updated_utc; + +ALTER TABLE stb_schedule_run + RENAME COLUMN run_start TO run_start_utc; + +ALTER TABLE stb_schedule_run + RENAME COLUMN status_time TO status_time_utc; + +ALTER TABLE stb_schedule_log_entry + RENAME COLUMN log_time TO log_time_utc; + +-- Update table version to 2 +UPDATE stb_schedule_table_version SET version = 2; + +-- Change run_once column from bit to varchar +ALTER TABLE stb_schedule + ALTER COLUMN run_once VARCHAR(100) NULL; diff --git a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1__Create_initial_tables.sql b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_2__Create_initial_tables.sql similarity index 74% rename from scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1__Create_initial_tables.sql rename to scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_2__Create_initial_tables.sql index 66e2b0a..07ec08e 100644 --- a/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_1__Create_initial_tables.sql +++ b/scheduledtask-db-sql/src/main/java/com/storebrand/scheduledtask/db/sql/V_2__Create_initial_tables.sql @@ -20,16 +20,16 @@ -- Table for keeping track of the ScheduledTask master locks -- Column lock_name: the name of the scheduler -- Column node_name: node of the server that currently has the lock --- Column lock_taken_time: when this lock was last taken --- Column lock_last_updated_time: used by the master node to keep the lock +-- Column lock_taken_time_utc: when this lock was last taken +-- Column lock_last_updated_time_utc: used by the master node to keep the lock -- --- The columns lock_taken_time and lock_last_updated_time is used to keep track on the master node +-- The columns lock_taken_time_utc and lock_last_updated_time_utc is used to keep track on the master node -- to see if he currently is actively keeping the lock and how long he has kept it. CREATE TABLE stb_schedule_master_locker ( lock_name VARCHAR(255) NOT NULL, node_name VARCHAR(255) NOT NULL, - lock_taken_time datetime2 NOT NULL, - lock_last_updated_time datetime2 NOT NULL, + lock_taken_time_utc datetime2 NOT NULL, + lock_last_updated_time_utc datetime2 NOT NULL, CONSTRAINT PK_lock_name PRIMARY KEY (lock_name) ); @@ -43,7 +43,7 @@ CREATE TABLE stb_schedule_table_version ( ); -- If changes are done to these tables this value should increase by one. -INSERT INTO stb_schedule_table_version (version) VALUES (1); +INSERT INTO stb_schedule_table_version (version) VALUES (2); -------------------------------------------------- -- stb_schedule @@ -51,20 +51,21 @@ INSERT INTO stb_schedule_table_version (version) VALUES (1); -- Table for keeping track of the ScheduledTasks -- Column schedule_name: the name of the schedule -- Column is_active: flag that informs if this schedule is active (IE is running or paused) --- Column run_once: flag that informs that this schedule should run immediately regardless of next_run +-- Column run_once: String that informs that this schedule should run immediately regardless of next_run_utc. Will specify +-- if it was triggered by a user or by the system. If this is null then we are running normal schedule. -- Column cron_expression: When null the default coded in the java file will be used. if set then tis is the override --- Column next_run: timestamp on when the schedule should be running next time --- Column last_updated: Timestamp when this row was last updated. IE when the last run was triggered. +-- Column next_run_utc: timestamp on when the schedule should be running next time +-- Column last_updated_utc: Timestamp when this row was last updated. IE when the last run was triggered. -- --- Note the last_updated may be set even if the is_active is false. This means the execution of the schedule is +-- Note the last_updated_utc may be set even if the is_active is false. This means the execution of the schedule is -- deactivated but it will be doing it's normal schedule "looping" CREATE TABLE stb_schedule ( schedule_name VARCHAR(255) NOT NULL, is_active BIT NOT NULL, - run_once BIT NOT NULL, + run_once VARCHAR (100) NULL, cron_expression VARCHAR(255) NULL, - next_run datetime2 NOT NULL, - last_updated datetime2 NOT NULL, + next_run_utc datetime2 NOT NULL, + last_updated_utc datetime2 NOT NULL, CONSTRAINT PK_schedule_name PRIMARY KEY (schedule_name) ); @@ -75,24 +76,24 @@ CREATE TABLE stb_schedule ( -- Column run_id: ID for the run. -- Column schedule_name: the name of the schedule -- Column hostname: host that this instance runs on. --- Column run_start: When this schedule was started. +-- Column run_start_utc: When this schedule was started. -- Column status state of the schedule run, should be one of ScheduleTaskImpl.State STARTED/FAILED/DISPATCHED/DONE -- Column status_msg: Some informing text that is connected to the state. -- Column status_stacktrace: Can only be set on STATUS = FAILED and can contain a stacktrace --- Column status_time: When this schedule state was set. +-- Column status_time_utc: When this schedule state was set. CREATE TABLE stb_schedule_run ( run_id BIGINT NOT NULL IDENTITY(1, 1), schedule_name VARCHAR(255) NOT NULL, hostname VARCHAR(255) NOT NULL, - run_start DATETIME2 NOT NULL, + run_start_utc DATETIME2 NOT NULL, status VARCHAR(250) NULL, status_msg VARCHAR(MAX) NULL, status_stacktrace VARCHAR(MAX) NULL, - status_time DATETIME2 NOT NULL, + status_time_utc DATETIME2 NOT NULL, CONSTRAINT PK_run_id PRIMARY KEY (run_id) ); -CREATE INDEX IX_stb_schedule_run_name_start_status ON stb_schedule_run (schedule_name, run_start DESC, status); +CREATE INDEX IX_stb_schedule_run_name_start_status ON stb_schedule_run (schedule_name, run_start_utc DESC, status); -------------------------------------------------- -- stb_schedule_log_entry @@ -102,13 +103,13 @@ CREATE INDEX IX_stb_schedule_run_name_start_status ON stb_schedule_run (schedule -- Column: run_id: ID for the run. Foreign Key is from the scheduleRun table. -- Column: log_msg: message that is logged for the run. -- Column: log_stacktrace: If set contains a stacktrace in addition to the log_msg. --- Column: log_time: timestamp on when this log was recorded. +-- Column: log_time_utc: timestamp on when this log was recorded. CREATE TABLE stb_schedule_log_entry ( log_id BIGINT NOT NULL IDENTITY(1, 1), run_id BIGINT NOT NULL, log_msg VARCHAR(MAX) NOT NULL, log_stacktrace VARCHAR(MAX) NULL, - log_time DATETIME2 NOT NULL, + log_time_utc DATETIME2 NOT NULL, CONSTRAINT PK_log_id PRIMARY KEY (log_id), CONSTRAINT FK_run_id FOREIGN KEY (run_id) REFERENCES stb_schedule_run (run_id) ); diff --git a/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/MasterLockRepositoryTest.java b/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/MasterLockRepositoryTest.java index a7dfa15..ee89072 100644 --- a/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/MasterLockRepositoryTest.java +++ b/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/MasterLockRepositoryTest.java @@ -34,6 +34,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.datasource.SingleConnectionDataSource; @@ -48,19 +50,20 @@ * @author Dag Bertelsen - dag.lennart.bertelsen@storebrand.no - dabe@dagbertelsen.com - 2021.03 */ public class MasterLockRepositoryTest { + private static final Logger log = LoggerFactory.getLogger(MasterLockRepositoryTest.class); private final JdbcTemplate _jdbcTemplate; private final DataSource _dataSource; private final ClockMock _clock = new ClockMock(); static final String MASTER_TABLE_CREATE_SQL = "CREATE TABLE " + MasterLockSqlRepository.MASTER_LOCK_TABLE + " ( " - + " lock_name VARCHAR(255) NOT NULL, " - + " node_name VARCHAR(255) NOT NULL, " - + " lock_taken_time datetime2 NOT NULL, " - + " lock_last_updated_time datetime2 NOT NULL, " - + " CONSTRAINT PK_lock_name PRIMARY KEY (lock_name) " - + " );"; + + " lock_name VARCHAR(255) NOT NULL, " + + " node_name VARCHAR(255) NOT NULL, " + + " lock_taken_time_utc DATETIME2 NOT NULL, " + + " lock_last_updated_time_utc DATETIME2 NOT NULL, " + + " CONSTRAINT PK_lock_name PRIMARY KEY (lock_name) " + + " );"; static final String SCHEDULE_TABLE_VERSION_CREATE_SQL = "CREATE TABLE " + TableInspector.TABLE_VERSION + " ( " - + " version int NOT NULL" + + " version INT NOT NULL" + " ) "; @@ -166,6 +169,28 @@ public void acquireLockThatIsTaken_fail() { assertEquals("test-node-1", currentLock.get().getNodeName()); } + /** + * Testing release lock when the lock is not acquired. + */ + @Test + public void releaseLock_ok() { + // :: Setup + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + master.tryCreateLock("acquiredLock", "test-node-1"); + boolean acquired1 = master.tryAcquireLock("acquiredLock", "test-node-1"); + + // :: Act + boolean releaseLock = master.releaseLock("acquiredLock", "test-node-1"); + + // :: Assert + assertTrue(acquired1); + assertTrue(releaseLock); + Optional currentLock = master.getLock("acquiredLock"); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + assertEquals(Instant.EPOCH, currentLock.get().getLockTakenTime()); + } + /** * Neither of the nodes should be able to acquire a lock if it where taken withing the last 5 min */ @@ -179,7 +204,7 @@ public void acquireLockShouldNotBeRequired_fail() { master.tryCreateLock("acquiredLock", "test-node-1"); master.tryAcquireLock("acquiredLock", "test-node-1"); _clock.setFixedClock(LocalDateTime.of(2021, 2, 28, 13, 22) - .atZone(ZoneId.systemDefault()).toInstant()); + .atZone(ZoneId.systemDefault()).toInstant()); // :: Act boolean inserted1 = master.tryAcquireLock("acquiredLock", "test-node-1"); @@ -197,8 +222,8 @@ public void acquireLockShouldNotBeRequired_fail() { } /** - * Test for a lock that where taken 6 min ago (1 min more than the time span that the lock can still be kept for - * the master node). Here no one should be able to take the lock due to it where last kept 5+ minutes ago. + * Test for a lock that where taken 6 min ago (1 min more than the time span that the lock can still be kept for the + * master node). Here no one should be able to take the lock due to it where last kept 5+ minutes ago. */ @Test public void acquireLockThatIsTaken6MinAgo_fail() { @@ -262,8 +287,231 @@ public void acquireLockThatIsTaken10MinAgo_ok() { assertEquals(secondAcquire, currentLock.get().getLockLastUpdatedTime()); assertEquals(secondAcquire, currentLock.get().getLockTakenTime()); } + + /** + * Test for daylight-saving time. Since the code is adjusting the times by subtracting 10/5 minutes from the current + * time, we should check that the time is not affected by the daylight-saving time. When we subtract 5 minutes from + * a time that just passed daylight-saving time the time should be 1 hour earlier than the current time (depending + * on the zone the code uses). + */ + @Test + public void from_dayLightSaving_should_not_affect_acquireLock_ok() { + // :: Setup + Instant initiallyAcquired = Instant.parse("2024-10-27T01:02:28.586Z"); + _clock.setFixedClock(initiallyAcquired); + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + master.tryCreateLock("acquiredLock", "test-node-1"); + + // :: Act + // first node acquires the lock + boolean inserted1 = master.tryAcquireLock("acquiredLock", "test-node-1"); + // move the clock 2 minutes ahead, this should not affect the lock time even if we are in daylight-saving time + _clock.setFixedClock(Instant.parse("2024-10-27T01:04:28.586Z")); + boolean inserted2 = master.tryAcquireLock("acquiredLock", "test-node-2"); + + // :: Assert + // First node should be able to acquire the lock. + assertTrue(inserted1); + // Node 2 should not be able to acquire the lock due to node 1 just took this one. + assertFalse(inserted2); + + Optional currentLock = master.getLock("acquiredLock"); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + assertEquals(initiallyAcquired, currentLock.get().getLockLastUpdatedTime()); + assertEquals(initiallyAcquired, currentLock.get().getLockTakenTime()); + } + + /** + * It's daylight-saving-time hours (autumn), and we should still be able to take the lock. Even during the hour + * when the clock jumps one hour back. + */ + @Test + public void acquire_lock_ok() { + // :: Setup + Instant initiallyAcquired = Instant.parse("2024-10-27T01:59:28.586Z"); + _clock.setFixedClock(initiallyAcquired); + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + master.tryCreateLock("acquiredLock", "test-node-1"); + + // :: Act + // first node acquires the lock + boolean inserted1 = master.tryAcquireLock("acquiredLock", "test-node-1"); + // move the clock 2 minutes ahead, this should not affect the lock time even if we are in daylight-saving time + Instant adjustedInstant = Instant.parse("2024-10-27T02:02:28.586Z"); + _clock.setFixedClock(adjustedInstant); + boolean kept = master.keepLock("acquiredLock", "test-node-1"); + + // :: Assert + // First node should be able to acquire the lock. + assertTrue(inserted1); + assertTrue(kept); + + Optional currentLock = master.getLock("acquiredLock"); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + log.error("### Lock.lastUpdatedTime: " + currentLock.get().getLockLastUpdatedTime() + ", LocalDateTime: " + currentLock.get().getLockLastUpdatedTime()); + assertEquals(adjustedInstant, currentLock.get().getLockLastUpdatedTime()); + assertEquals(initiallyAcquired, currentLock.get().getLockTakenTime()); + } + + // ===== Keep lock ============================================================================ + /** + * Node1 manages to get the lock, then do a keep lock update after 3 minutes. Node 2 should not be allowed to + * acquire the lock during this time. This occurs during the hour when daylight-saving is adjusted to winter time. + */ + @Test + public void keepLockAfterAcquired_from_daylightSavingTime_ok() { + // :: Setup + // We use instant here due to we want to have control on what "hour" the dts is in. At this time, + // the clock moves one our back at 3:00 so there is in LocalDateTime two 02:00 but in zulu time there is one 00:00 + // and one 01:00 that is the same time in localDateTime + Instant initiallyAcquired = Instant.parse("2024-10-27T01:02:28.586Z"); + _clock.setFixedClock(initiallyAcquired); + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + master.tryCreateLock("acquiredLock", "test-node-1"); + boolean acquiredLock = master.tryAcquireLock("acquiredLock", "test-node-1"); + + // :: Act + // move the clock 2 minutes ahead, this should not affect the lock time even if we are in daylight-saving time + Instant updatedTime = Instant.parse("2024-10-27T01:04:28.586Z"); + _clock.setFixedClock(updatedTime); + boolean keepLock = master.keepLock("acquiredLock", "test-node-1"); + + // :: Assert - locks where recently acquired, so node 1 should be able to keep the lock + assertTrue(acquiredLock); + assertTrue(keepLock); + Optional currentLock = master.getLock("acquiredLock"); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + assertEquals(updatedTime, currentLock.get().getLockLastUpdatedTime()); + assertEquals(initiallyAcquired, currentLock.get().getLockTakenTime()); + } + + /** + * We start at 23:00 and keep the lock for 250 minutes by 1-minute interval. This tests when we are in the + * winter time and no daylight-saving time is adjusted. Node 2 should not be able to acquire the lock during this time. + */ + @Test + public void keepLockAfterAcquired_slide_wintertime() { + keepLockSlide(Instant.parse("2024-01-01T23:00:00.000Z")); + } + + /** + * We start at 23:00 and keep the lock for 250 minutes by 1-minute interval. This tests it when we are in the + * spring when we adjust the clock to daylight-saving time by moving the clock back 1 hour at 03:00 to 02:00. + * We should be able to keep the lock in this entire period. + * Node 2 should not be able to acquire the lock during this time. + */ + @Test + public void keepLockAfterAcquired_slide_springTransition() { + // an hour before midnight 2024-03-31 + keepLockSlide(Instant.parse("2024-03-30T23:00:00.000Z")); + } + + /** + * We start at 23:00 and keep the lock for 250 minutes by 1-minute interval. This tests when we are in the summer + * time and no daylight-saving time is adjusted. We should be able to keep the lock on this entire period. + * Node 2 should not be able to acquire the lock during this time. + */ + @Test + public void keepLockAfterAcquired_slide_summertime() { + keepLockSlide(Instant.parse("2024-07-01T23:00:00.000Z")); + } + + /** + * We start at 23:00 and keep the lock for 250 minutes by 1-minute interval. This tests it when we are in the autumn + * transition when we adjust the clock to daylight-saving time by moving the clock back 1 hour at 03:00 to 02:00. + * At this time, we will get the hours between 2 and 3 twice. We should be able to keep the lock on this entire period. + * Node 2 should not be able to acquire the lock during this time. + */ + @Test + public void keepLockAfterAcquired_slide_autumnTransition() { + // an hour before midnight 2024-10-27 + keepLockSlide(Instant.parse("2024-10-26T23:00:00.000Z")); + } + + /** + * Helper method to test the keep lock for 250 minutes by 1-minute interval. + * Node 2 should not be able to acquire the lock during this time. + */ + public void keepLockSlide(Instant start) { + // :: Setup + _clock.setFixedClock(start); + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + boolean created = master.tryCreateLock("acquiredLock", "test-node-1"); + boolean tryAcquired = master.tryAcquireLock("acquiredLock", "test-node-1"); + boolean kept = master.keepLock("acquiredLock", "test-node-1"); + + assertTrue(created, "Lock should initially be created"); + assertTrue(tryAcquired, "Lock should initially be tryAcquired"); + assertTrue(kept, "Lock should initially be kept"); + + // :: Act + Instant slide = start; + for (int t = 0; t < 250; t++) { + _clock.setFixedClock(slide); + boolean wholeHour = slide.atZone(ZoneId.systemDefault()).getMinute() == 0; + if (wholeHour) { + log.info("\n\n!!!!!!!!! Whole hour: " + slide + ", LocalDateTime: " + LocalDateTime.ofInstant(slide, ZoneId.systemDefault())); + } + log.info("### Slide time: " + slide + ", LocalDateTime: " + LocalDateTime.ofInstant(slide, ZoneId.systemDefault())); + boolean acquireLockNode2 = master.tryAcquireLock("acquiredLock", "test-node-2"); + boolean keepLockUpdatesNode1 = master.keepLock("acquiredLock", "test-node-1"); + boolean keepLockNode2 = master.keepLock("acquiredLock", "test-node-2"); + assertTrue(keepLockUpdatesNode1, "Node 1 should be able to keep the lock"); + assertFalse(acquireLockNode2, "Node 2 should not be able to acquire the lock"); + assertFalse(keepLockNode2, "Node 2 should not be able to keep the lock"); + + Optional currentLock = master.getLock("acquiredLock"); + log.info(".. ### Lock.lastUpdatedTime: " + currentLock.get().getLockLastUpdatedTime() + ", LocalDateTime: " + currentLock.get().getLockLastUpdatedTime()); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + + assertEquals(slide, currentLock.get().getLockLastUpdatedTime()); + assertEquals(start, currentLock.get().getLockTakenTime()); + // move the slide 1 minute ahead + slide = slide.plus(1, ChronoUnit.MINUTES); + log.info("\n--------------------------------------\n\n"); + } + } + + + /** + * Node1 manages to get the lock, then do a keep lock update after 3 minutes. Node 2 should not be allowed to + * acquire the lock during this time. This occurs during the hour when wintertime is adjusted to daylight-saving. + */ + @Test + public void keepLockAfterAcquired_to_daylightSavingTime_ok() { + // :: Setup + // We use instant here due to we want to have control on what "hour" the dts is in. + Instant initiallyAcquired = Instant.parse("2024-03-31T00:59:00Z"); + _clock.setFixedClock(initiallyAcquired); + MasterLockSqlRepository master = new MasterLockSqlRepository(_dataSource, _clock); + master.tryCreateLock("acquiredLock", "test-node-1"); + master.tryAcquireLock("acquiredLock", "test-node-1"); + + // :: Act + // move the clock 2 minutes ahead, this should not affect the lock time even if we are in daylight-saving time + Instant keepLockTime = Instant.parse("2024-03-31T01:04:00Z"); + _clock.setFixedClock(keepLockTime); + boolean keepLockUpdatesNode1 = master.keepLock("acquiredLock", "test-node-1"); + boolean acquireLockNode2 = master.tryAcquireLock("acquiredLock", "test-node-2"); + + // :: Assert + assertTrue(keepLockUpdatesNode1); + assertFalse(acquireLockNode2); + Optional currentLock = master.getLock("acquiredLock"); + assertTrue(currentLock.isPresent()); + assertEquals("test-node-1", currentLock.get().getNodeName()); + assertNotEquals(initiallyAcquired, currentLock.get().getLockLastUpdatedTime()); + assertEquals(keepLockTime, currentLock.get().getLockLastUpdatedTime()); + assertEquals(initiallyAcquired, currentLock.get().getLockTakenTime()); + } + + /** * Node1 manages to get the lock, then do a keep lock update after 3 minutes. Node 2 should not be allowed to * acquire the lock during this time. @@ -340,10 +588,9 @@ public void multipleKeepLockAfterAcquired_ok() { } /** - * Node that acquired the lock manages to keep it multiple time and we verify that after it has kept it for more - * than 10 min. During - * this time we test if the node2 can also do the keepLock, it should not be able to aquire the lock due to first - * node has done keepLock during these 10 minutes. + * Node that acquired the lock manages to keep it multiple time, and we verify that after it has kept it for more + * than 10 min. During this time we test if the node2 can also do the keepLock, it should not be able to acquire the + * lock due to the first node has done keepLock during these 10 minutes. */ @Test public void multipleKeepLockAfterAcquiredOtherNodeShouldNotBeAllowedToDoKeepLock_ok() { @@ -519,6 +766,7 @@ public void acquiringLockShouldNotBePossibleBefore10MinutesAfterCreatingMissingL } // ===== Helpers ============================================================================== + /** Fixed clock to replace real clock in unit/integration tests. */ public static class ClockMock extends Clock { private Clock _clock = Clock.systemDefaultZone(); diff --git a/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepositoryTest.java b/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepositoryTest.java index 3b59ed3..4aabb50 100644 --- a/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepositoryTest.java +++ b/scheduledtask-db-sql/src/test/java/com/storebrand/scheduledtask/db/sql/ScheduledTaskSqlRepositoryTest.java @@ -42,12 +42,13 @@ import com.storebrand.scheduledtask.ScheduledTask.Criticality; import com.storebrand.scheduledtask.ScheduledTask.Recovery; import com.storebrand.scheduledtask.ScheduledTaskConfig; +import com.storebrand.scheduledtask.ScheduledTaskRegistry; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.State; -import com.storebrand.scheduledtask.db.sql.MasterLockRepositoryTest.ClockMock; -import com.storebrand.scheduledtask.ScheduledTaskRegistry; import com.storebrand.scheduledtask.db.ScheduledTaskRepository.ScheduledRunDto; +import com.storebrand.scheduledtask.db.sql.MasterLockRepositoryTest.ClockMock; /** * Tests for {@link ScheduledTaskSqlRepository} @@ -62,10 +63,10 @@ public class ScheduledTaskSqlRepositoryTest { "CREATE TABLE " + ScheduledTaskSqlRepository.SCHEDULE_TASK_TABLE + " ( " + " schedule_name VARCHAR(255) NOT NULL, " + " is_active BIT NOT NULL, " - + " run_once BIT NOT NULL, " + + " run_once VARCHAR (100) NULL, " + " cron_expression VARCHAR(255) NULL, " - + " next_run DATETIME2 NOT NULL, " - + " last_updated DATETIME2 NOT NULL, " + + " next_run_utc DATETIME2 NOT NULL, " + + " last_updated_utc DATETIME2 NOT NULL, " + " CONSTRAINT PK_schedule_name PRIMARY KEY (schedule_name) " + " );"; @@ -74,16 +75,16 @@ public class ScheduledTaskSqlRepositoryTest { + "run_id BIGINT NOT NULL IDENTITY(1, 1), " + " schedule_name VARCHAR(255) NOT NULL, " + " hostname VARCHAR(255) NOT NULL, " - + " run_start DATETIME2 NOT NULL, " + + " run_start_utc DATETIME2 NOT NULL, " + " status VARCHAR(250) NULL, " + " status_msg VARCHAR(MAX) NULL, " + " status_stacktrace VARCHAR(MAX) NULL, " - + " status_time DATETIME2 NOT NULL, " + + " status_time_utc DATETIME2 NOT NULL, " + " CONSTRAINT PK_run_id PRIMARY KEY (run_id) " + " );"; static final String SCHEDULE_RUN_INDEX_CREATE_SQL = "CREATE INDEX IX_stb_schedule_run_name_start_status" - + " ON stb_schedule_run (schedule_name, run_start DESC, status);"; + + " ON stb_schedule_run (schedule_name, run_start_utc DESC, status);"; static final String SCHEDULE_LOG_ENTRY_CREATE_SQL = "CREATE TABLE " + ScheduledTaskSqlRepository.SCHEDULE_LOG_ENTRY_TABLE + " ( " @@ -91,7 +92,7 @@ public class ScheduledTaskSqlRepositoryTest { + " run_id BIGINT NOT NULL, " + " log_msg VARCHAR(MAX) NOT NULL, " + " log_stacktrace VARCHAR(MAX) NULL, " - + " log_time DATETIME2 NOT NULL, " + + " log_time_utc DATETIME2 NOT NULL, " + " CONSTRAINT PK_log_id PRIMARY KEY (log_id)," + " CONSTRAINT FK_run_id FOREIGN KEY (run_id) REFERENCES stb_schedule_run (run_id) " + " );"; @@ -294,7 +295,7 @@ public void setRunOnce() { LocalDateTime updateTime = LocalDateTime.of(2021, 3, 3, 12, 12); _clock.setFixedClock(updateTime); Optional beforeSettingRunOnce = schedulerRep.getSchedule("test-schedule"); - schedulerRep.setRunOnce("test-schedule", true); + schedulerRep.setRunOnce("test-schedule", RunOnce.PROGRAMMATIC); // :: Assert assertTrue(beforeSettingRunOnce.isPresent()); @@ -307,6 +308,8 @@ public void setRunOnce() { assertEquals(insertTimeInstant, beforeSettingRunOnce.get().getLastUpdated()); assertNull(afterSetRunOnce.get().getOverriddenCronExpression().orElse(null)); assertEquals(insertTimeInstant, afterSetRunOnce.get().getLastUpdated()); + assertFalse(beforeSettingRunOnce.get().getRunOnce().isPresent()); + assertEquals(RunOnce.PROGRAMMATIC, afterSetRunOnce.get().getRunOnce().orElse(null)); } @Test diff --git a/scheduledtask-healthcheck/src/main/java/com/storebrand/scheduledtask/healthcheck/ScheduledTaskHealthCheck.java b/scheduledtask-healthcheck/src/main/java/com/storebrand/scheduledtask/healthcheck/ScheduledTaskHealthCheck.java index 44879d9..47dc488 100644 --- a/scheduledtask-healthcheck/src/main/java/com/storebrand/scheduledtask/healthcheck/ScheduledTaskHealthCheck.java +++ b/scheduledtask-healthcheck/src/main/java/com/storebrand/scheduledtask/healthcheck/ScheduledTaskHealthCheck.java @@ -74,6 +74,7 @@ public class ScheduledTaskHealthCheck implements ScheduledTaskListener { private volatile CheckSpecification _checkSpecification; + @SuppressWarnings("this-escape") @SuppressFBWarnings("EI_EXPOSE_REP2") public ScheduledTaskHealthCheck(ScheduledTaskRegistry scheduledTaskRegistry, HealthCheckRegistry healthCheckRegistry, Clock clock) { diff --git a/scheduledtask-localinspect/src/main/java/com/storebrand/scheduledtask/localinspect/LocalHtmlInspectScheduler.java b/scheduledtask-localinspect/src/main/java/com/storebrand/scheduledtask/localinspect/LocalHtmlInspectScheduler.java index 852e365..73ce33d 100644 --- a/scheduledtask-localinspect/src/main/java/com/storebrand/scheduledtask/localinspect/LocalHtmlInspectScheduler.java +++ b/scheduledtask-localinspect/src/main/java/com/storebrand/scheduledtask/localinspect/LocalHtmlInspectScheduler.java @@ -31,7 +31,7 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; - import java.util.Comparator; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -41,6 +41,7 @@ import com.storebrand.scheduledtask.ScheduledTaskRegistry; import com.storebrand.scheduledtask.ScheduledTaskRegistry.LogEntry; import com.storebrand.scheduledtask.ScheduledTaskRegistry.MasterLock; +import com.storebrand.scheduledtask.ScheduledTaskRegistry.RunOnce; import com.storebrand.scheduledtask.ScheduledTaskRegistry.Schedule; import com.storebrand.scheduledtask.ScheduledTaskRegistry.ScheduleRunContext; import com.storebrand.scheduledtask.ScheduledTaskRegistry.State; @@ -814,7 +815,7 @@ private void toggleActive(String scheduleName) { */ private void triggerSchedule(String scheduleName) { _scheduledTaskRegistry.getScheduledTasks().computeIfPresent(scheduleName, (ignored, scheduled) -> { - scheduled.runNow(); + scheduled.runNow(RunOnce.MONITOR); return scheduled; }); } diff --git a/scheduledtask-spring/src/main/java/com/storebrand/scheduledtask/spring/ScheduledTaskRegistryFactory.java b/scheduledtask-spring/src/main/java/com/storebrand/scheduledtask/spring/ScheduledTaskRegistryFactory.java index a8c568a..8fe4ea1 100644 --- a/scheduledtask-spring/src/main/java/com/storebrand/scheduledtask/spring/ScheduledTaskRegistryFactory.java +++ b/scheduledtask-spring/src/main/java/com/storebrand/scheduledtask/spring/ScheduledTaskRegistryFactory.java @@ -76,7 +76,12 @@ public class ScheduledTaskRegistryFactory extends AbstractFactoryBean