Skip to content

Commit

Permalink
fix: Revert change in behavior for MariaDB and MySQL with regards to …
Browse files Browse the repository at this point in the history
…time zone assumptions for persisted timestamps (#481)

Change back to default behavior for MySQL and MariaDB with regards to
timestamp persistence and assumptions. It is recommended to persist
timestamps in UTC for users of MariaDB and MySQL, but db-scheduler will
not enforce it as default, since it in reality requires a controlled
upgrade.

Ideally the upgrade-process should be:
1. All instances of the scheduler should be shutdown
2. Timestamps in the database-table migrated to UTC
3. Configuration updated to use `alwaysPersistTimestampInUTC()`
4. Instances started again
  • Loading branch information
kagkarlsson authored Apr 18, 2024
1 parent 823c9d2 commit b5578ef
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 27 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,10 @@ How often the scheduler checks the database for due executions. Default `10s`.<b
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.
Currently, only PostgreSQL and Oracle rely on the database preserving time zone information,
other databases store timestamps in UTC. **NB:** For backwards compatibility, the default behavior
For such cases, use this setting to always store Instants in UTC.
PostgreSQL and Oracle-schemas is tested to preserve zone-information. **MySQL** and **MariaDB**-schemas
_does not_ and should use this setting.
**NB:** For backwards compatibility, the default behavior
for "unknown" databases is to assume the database preserves time zone. For "known" databases,
see the class `AutodetectJdbcCustomization`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ public class DbSchedulerProperties {
/** What polling-strategy to use. Valid values are: FETCH,LOCK_AND_FETCH */
private PollingStrategyConfig.Type pollingStrategy =
SchedulerBuilder.DEFAULT_POLLING_STRATEGY.type;

/**
* The limit at which more executions are fetched from the database after fetching a full batch.
*/
private double pollingStrategyLowerLimitFractionOfThreads =
SchedulerBuilder.DEFAULT_POLLING_STRATEGY.lowerLimitFractionOfThreads;

/**
* For Type=FETCH, the number of due executions fetched from the database in each batch.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
public class AutodetectJdbcCustomization implements JdbcCustomization {

private static final Logger LOG = LoggerFactory.getLogger(AutodetectJdbcCustomization.class);
private static final Logger SILENCABLE_LOG =
LoggerFactory.getLogger(LOG.getName() + ".utc_warning");
public static final String MICROSOFT_SQL_SERVER = "Microsoft SQL Server";
public static final String POSTGRESQL = "PostgreSQL";
public static final String ORACLE = "Oracle";
Expand All @@ -48,6 +50,9 @@ public AutodetectJdbcCustomization(DataSource dataSource, boolean persistTimesta

if (databaseProductName.equals(MICROSOFT_SQL_SERVER)) {
LOG.info("Using MSSQL jdbc-overrides.");
if (persistTimestampInUTC) {
LOG.info("Redundant 'persistTimestampInUTC' setting. MSSQL will always persist in UTC.");
}
detectedCustomization = new MssqlJdbcCustomization(true);
} else if (databaseProductName.equals(POSTGRESQL)) {
LOG.info("Using PostgreSQL jdbc-overrides.");
Expand All @@ -57,25 +62,26 @@ public AutodetectJdbcCustomization(DataSource dataSource, boolean persistTimesta
detectedCustomization = new OracleJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.contains(MARIADB)) {
LOG.info("Using MariaDB jdbc-overrides.");
detectedCustomization = new MariaDBJdbcCustomization(true);
logWarningIfNotUTC("MariaDB", persistTimestampInUTC);
detectedCustomization = new MariaDBJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.contains(MYSQL)) {
int databaseMajorVersion = c.getMetaData().getDatabaseMajorVersion();
String dbVersion = c.getMetaData().getDatabaseProductVersion();
logWarningIfNotUTC("MySQL", persistTimestampInUTC);
if (databaseMajorVersion >= 8) {
LOG.info("Using MySQL jdbc-overrides version 8 and later. (v {})", dbVersion);
detectedCustomization = new MySQL8JdbcCustomization(true);
detectedCustomization = new MySQL8JdbcCustomization(persistTimestampInUTC);
} else {
LOG.info("Using MySQL jdbc-overrides for version older than 8. (v {})", dbVersion);
detectedCustomization = new MySQLJdbcCustomization(true);
detectedCustomization = new MySQLJdbcCustomization(persistTimestampInUTC);
}
} else {
if (persistTimestampInUTC) {
LOG.info(
"No database-specific jdbc-overrides applied. Behavior overridden to always store "
+ "timestamps in zone UTC");
} else {
Logger silencableLogger = LoggerFactory.getLogger(LOG.getName() + ".utc_warning");
silencableLogger.warn(
SILENCABLE_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 Expand Up @@ -151,4 +157,18 @@ public String createSelectDueQuery(String tableName, int limit, String andCondit
public String getName() {
return jdbcCustomization.getName();
}

private void logWarningIfNotUTC(String database, boolean persistTimestampInUTC) {
if (!persistTimestampInUTC) {
SILENCABLE_LOG.warn(
"{}-schema does not support persistent timezones. "
+ "It is recommended to store time in UTC to avoid issues with for example DST. "
+ "For first time users, use setting 'alwaysPersistTimestampInUtc' to achieve this. "
+ "Users upgrading from a version prior to v14.0.0 can either silence this logger, "
+ "or perform a controlled upgrade to UTC timestamps. All old instances "
+ "of the scheduler must be stopped and timestamps migrated to UTC before starting "
+ "again, using 'alwaysPersistTimestampInUtc=true'.",
database);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ public class MariaDBJdbcCustomization extends DefaultJdbcCustomization {

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 @@ -23,12 +23,6 @@ public class MySQL8JdbcCustomization extends DefaultJdbcCustomization {

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 @@ -21,12 +21,6 @@ public class MySQLJdbcCustomization extends DefaultJdbcCustomization {

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 @@ -39,6 +39,7 @@ public class SchedulerTest {

@RegisterExtension
public EmbeddedPostgresqlExtension postgres = new EmbeddedPostgresqlExtension();

// @RegisterExtension
// public ChangeLogLevelsExtension changeLogLevels = new ChangeLogLevelsExtension(
// new ChangeLogLevelsExtension.LogLevelOverride("com.github.kagkarlsson.scheduler",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class PostgresqlCompatibilityTest extends CompatibilityTest {

@RegisterExtension
public EmbeddedPostgresqlExtension postgres = new EmbeddedPostgresqlExtension();

// Enable if test gets flaky!
// @RegisterExtension
// public ChangeLogLevelsExtension changeLogLevels = new ChangeLogLevelsExtension(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class ExecutorPoolTest {
public EmbeddedPostgresqlExtension postgres = new EmbeddedPostgresqlExtension();

@RegisterExtension public StopSchedulerExtension stopScheduler = new StopSchedulerExtension();

// Enable if test gets flaky!
// @RegisterExtension
// public ChangeLogLevelsExtension changeLogLevels = new ChangeLogLevelsExtension(
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,15 @@
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<version>2.36.0</version>
<version>2.43.0</version>
<configuration>
<java>
<includes>
<include>src/main/java/**/*.java</include> <!-- Check application code -->
<include>src/test/java/**/*.java</include> <!-- Check application tests code -->
</includes>
<googleJavaFormat>
<version>1.16.0</version>
<version>1.17.0</version>
<style>GOOGLE</style>
</googleJavaFormat>
<importOrder />
Expand Down

0 comments on commit b5578ef

Please sign in to comment.