Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Revert change in behavior for MariaDB and MySQL with regards to time zone assumptions for persisted timestamps #481

Merged
merged 6 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading