Skip to content

Commit

Permalink
feat: MySQL, MariaDB always store timestamps in UTC (#471)
Browse files Browse the repository at this point in the history
* **BREAKING CHANGE:** Since **MySQL** and **MariaDB** does not have a
datatype for storing timestamps with time zones, start defaulting to
storing them in UTC to avoid issues with DST
* **BREAKING CHANGE:** Change **Oracle** schema to use a datatype that
supports time zones
* New config-option to override default timestamp persistence behavior
for PostgreSQL and Oracle to UTC
* mysql-jdbc `v12.4.2.jre8`
* mariadb-java-client `v3.3.2`
* testcontainers `v1.19.7`
  • Loading branch information
kagkarlsson authored Mar 11, 2024
1 parent bb17380 commit d422cfb
Show file tree
Hide file tree
Showing 19 changed files with 181 additions and 62 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ Number of threads. Default `10`.
:gear: `.pollingInterval(Duration)`<br/>
How often the scheduler checks the database for due executions. Default `10s`.<br/>

:gear: `.alwaysPersistTimestampInUTC()`<br/>
The Scheduler assumes that columns for persisting timestamps persist `Instant`s, not `LocalDateTime`s,
i.e. somehow tie the timestamp to a zone. However, some databases have limited support for such types
(which has no zone information) or other quirks, making "always store in UTC" a better alternative.
Databases that always persist in UTC are: ...TODO
To force ..

:gear: `.enableImmediateExecution()`<br/>
If this is enabled, the scheduler will attempt to hint to the local `Scheduler` that there are executions to be executed after they are scheduled to
run `now()`, or a time in the past. **NB:** If the call to `schedule(..)`/`reschedule(..)` occur from within a transaction, the scheduler might attempt to run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ public Scheduler scheduler(DbSchedulerCustomizer customizer, StatsRegistry regis
.jdbcCustomization()
.orElse(new AutodetectJdbcCustomization(transactionalDataSource)));

if (config.isAlwaysPersistTimestampInUtc()) {
builder.alwaysPersistTimestampInUTC();
}

if (config.isImmediateExecutionEnabled()) {
builder.enableImmediateExecution();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ public class DbSchedulerProperties {
@DurationUnit(SECONDS)
private Duration shutdownMaxWait = SchedulerBuilder.SHUTDOWN_MAX_WAIT;

/**
* Store timestamps in UTC timezone even though the schema supports storing timezone information
*/
private boolean alwaysPersistTimestampInUtc = false;

/** Which log level to use when logging task failures. Defaults to {@link LogLevel#DEBUG}. */
private LogLevel failureLoggerLevel = SchedulerBuilder.DEFAULT_FAILURE_LOG_LEVEL;

Expand Down Expand Up @@ -228,4 +233,12 @@ public void setPollingStrategyUpperLimitFractionOfThreads(
double pollingStrategyUpperLimitFractionOfThreads) {
this.pollingStrategyUpperLimitFractionOfThreads = pollingStrategyUpperLimitFractionOfThreads;
}

public boolean isAlwaysPersistTimestampInUtc() {
return alwaysPersistTimestampInUtc;
}

public void setAlwaysPersistTimestampInUtc(boolean alwaysPersistTimestampInUTC) {
this.alwaysPersistTimestampInUtc = alwaysPersistTimestampInUTC;
}
}
4 changes: 2 additions & 2 deletions db-scheduler/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@
<dependency>
<groupId>com.microsoft.sqlserver</groupId>
<artifactId>mssql-jdbc</artifactId>
<version>8.2.1.jre8</version>
<version>12.4.2.jre8</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -287,7 +287,7 @@
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<version>3.1.4</version>
<version>3.3.2</version>
<scope>test</scope>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public class SchedulerBuilder {
protected LogLevel logLevel = DEFAULT_FAILURE_LOG_LEVEL;
protected boolean logStackTrace = LOG_STACK_TRACE_ON_FAILURE;
private boolean registerShutdownHook = false;
private boolean alwaysPersistTimestampInUTC = false;

public SchedulerBuilder(DataSource dataSource, List<Task<?>> knownTasks) {
this.dataSource = dataSource;
Expand Down Expand Up @@ -156,6 +157,11 @@ public SchedulerBuilder jdbcCustomization(JdbcCustomization jdbcCustomization) {
return this;
}

public SchedulerBuilder alwaysPersistTimestampInUTC() {
this.alwaysPersistTimestampInUTC = true;
return this;
}

public SchedulerBuilder shutdownMaxWait(Duration shutdownMaxWait) {
this.shutdownMaxWait = shutdownMaxWait;
return this;
Expand Down Expand Up @@ -208,7 +214,8 @@ public Scheduler build() {
final TaskResolver taskResolver = new TaskResolver(statsRegistry, clock, knownTasks);
final JdbcCustomization jdbcCustomization =
ofNullable(this.jdbcCustomization)
.orElseGet(() -> new AutodetectJdbcCustomization(dataSource));
.orElseGet(
() -> new AutodetectJdbcCustomization(dataSource, alwaysPersistTimestampInUTC));
final JdbcTaskRepository schedulerTaskRepository =
new JdbcTaskRepository(
dataSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public AutodetectJdbcCustomization(DataSource dataSource, boolean persistTimesta

if (databaseProductName.equals(MICROSOFT_SQL_SERVER)) {
LOG.info("Using MSSQL jdbc-overrides.");
detectedCustomization = new MssqlJdbcCustomization(persistTimestampInUTC);
detectedCustomization = new MssqlJdbcCustomization(true);
} else if (databaseProductName.equals(POSTGRESQL)) {
LOG.info("Using PostgreSQL jdbc-overrides.");
detectedCustomization = new PostgreSqlJdbcCustomization(false, persistTimestampInUTC);
Expand All @@ -57,16 +57,27 @@ public AutodetectJdbcCustomization(DataSource dataSource, boolean persistTimesta
detectedCustomization = new OracleJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.contains(MARIADB)) {
LOG.info("Using MariaDB jdbc-overrides.");
detectedCustomization = new MariaDBJdbcCustomization(persistTimestampInUTC);
detectedCustomization = new MariaDBJdbcCustomization(true);
} else if (databaseProductName.contains(MYSQL)) {
int databaseMajorVersion = c.getMetaData().getDatabaseMajorVersion();
String dbVersion = c.getMetaData().getDatabaseProductVersion();
if (databaseMajorVersion >= 8) {
LOG.info("Using MySQL jdbc-overrides version 8 and later. (v {})", dbVersion);
detectedCustomization = new MySQL8JdbcCustomization(persistTimestampInUTC);
detectedCustomization = new MySQL8JdbcCustomization(true);
} else {
LOG.info("Using MySQL jdbc-overrides for version older than 8. (v {})", dbVersion);
detectedCustomization = new MySQLJdbcCustomization(persistTimestampInUTC);
detectedCustomization = new MySQLJdbcCustomization(true);
}
} else {
if (persistTimestampInUTC) {
LOG.info(
"No database-specific jdbc-overrides applied. Behavior overridden to always store "
+ "timestamps in zone UTC");
} else {
LOG.warn(
"No database-specific jdbc-overrides applied. Assuming time-related columns "
+ "to be of type compatibe with 'TIMESTAMP WITH TIME ZONE', i.e. zone is persisted."
+ " If not, consider overriding to always UTC via '.alwaysPersistTimestampInUTC()'.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import java.util.TimeZone;

public class DefaultJdbcCustomization implements JdbcCustomization {
private static final Calendar UTC = GregorianCalendar.getInstance(TimeZone.getTimeZone("CET"));
public static final Calendar UTC = GregorianCalendar.getInstance(TimeZone.getTimeZone("UTC"));
private final boolean persistTimestampInUTC;

public DefaultJdbcCustomization(boolean persistTimestampInUTC) {
Expand All @@ -36,10 +36,15 @@ public DefaultJdbcCustomization(boolean persistTimestampInUTC) {

@Override
public void setInstant(PreparedStatement p, int index, Instant value) throws SQLException {
if (value == null) {
p.setTimestamp(index, null);
return;
}

if (persistTimestampInUTC) {
p.setTimestamp(index, value != null ? Timestamp.from(value) : null, UTC);
p.setTimestamp(index, Timestamp.from(value), UTC);
} else {
p.setTimestamp(index, value != null ? Timestamp.from(value) : null);
p.setTimestamp(index, Timestamp.from(value));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,20 @@

import static com.github.kagkarlsson.scheduler.jdbc.Queries.selectForUpdate;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class MariaDBJdbcCustomization extends DefaultJdbcCustomization {
private static final Logger LOG = LoggerFactory.getLogger(MariaDBJdbcCustomization.class);

public MariaDBJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
if (!persistTimestampInUTC) {
LOG.warn(
"{} does not support persistent timezones. "
+ "It is recommended to store time in UTC to avoid issues with for example DST",
getClass().getName());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,36 +15,31 @@

import static com.github.kagkarlsson.scheduler.jdbc.Queries.selectForUpdate;

import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.Calendar;
import java.util.TimeZone;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class MssqlJdbcCustomization extends DefaultJdbcCustomization {
private static final Logger LOG = LoggerFactory.getLogger(MssqlJdbcCustomization.class);

public MssqlJdbcCustomization() {
super(false);
super(true);
}

public MssqlJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
if (!persistTimestampInUTC) {
LOG.warn(
"{} must explicitly specify timezone when persisting a timestamp. "
+ "Persisting timestamp with undefined timezone is not recommended and will likely cause issues",
getClass().getName());
}
}

@Override
public String getName() {
return "MSSQL";
}

@Override
public void setInstant(PreparedStatement p, int index, Instant value) throws SQLException {
p.setTimestamp(
index,
value != null ? Timestamp.from(value) : null,
Calendar.getInstance(TimeZone.getTimeZone("UTC")));
}

@Override
public boolean supportsGenericLockAndFetch() {
// Currently supported, but not recommended because of deadlock issues
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,20 @@

import static com.github.kagkarlsson.scheduler.jdbc.Queries.selectForUpdate;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class MySQL8JdbcCustomization extends DefaultJdbcCustomization {
private static final Logger LOG = LoggerFactory.getLogger(MySQL8JdbcCustomization.class);

public MySQL8JdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
if (!persistTimestampInUTC) {
LOG.warn(
"{} does not support persistent timezones. "
+ "It is recommended to store time in UTC to avoid issues with for example DST",
getClass().getName());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,20 @@
*/
package com.github.kagkarlsson.scheduler.jdbc;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class MySQLJdbcCustomization extends DefaultJdbcCustomization {
private static final Logger LOG = LoggerFactory.getLogger(MySQLJdbcCustomization.class);

public MySQLJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
if (!persistTimestampInUTC) {
LOG.warn(
"{} does not support persistent timezones. "
+ "It is recommended to store time in UTC to avoid issues with for example DST",
getClass().getName());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@
public class PostgreSqlJdbcCustomization extends DefaultJdbcCustomization {
private final boolean useGenericLockAndFetch;

public PostgreSqlJdbcCustomization() {
this(false, false);
}

public PostgreSqlJdbcCustomization(
boolean useGenericLockAndFetch, boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ public static void sleep(int millis) {
public static void retryOnFailed(int retryTimes, Runnable r) {
try {
r.run();
} catch (RuntimeException e) {
} catch (RuntimeException | AssertionError e) {
if (retryTimes == 0) {
throw e;
} else {
LOG.info("Retrying test after failure.");
LOG.info("Retrying test after failure.", e);
retryOnFailed(retryTimes - 1, r);
}
}
Expand Down
Loading

0 comments on commit d422cfb

Please sign in to comment.