-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
SAK-48958 Some dates are stored in the database as GMT/UTC with mysql-connector/j 5.1.47+ #11966
Conversation
…-connector/j 5.1.47+
# SAK-48958 | ||
# When using mysql-connector/j 5.1.47+, configure the timezone, usually your server's localtime | ||
# DEFAULT: GMT | ||
# database.timezone= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is already -Duser.timezone java property. why add another?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that preferable to sakai.properties? do most institutions run tomcat with that option? (we include it). If so, I can make that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's essential to run Tomcat with the java property if your server uses UTC and your users are in a different timezone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe an important recommendation has always been to make sure that your tomcat and database configuration are in the same timezone. There have been issues like this that appear when storing dates with no tz info and mysql has changed stances on choosing to store using the mysql servers tz when the jdbc spec says it should be using the (jvm's tz) tz on the connector.
I think keeping Sakai code out of this and using configuration is probably more ideal.
This is the issue that was addressed in 5.1.47 https://bugs.mysql.com/bug.php?id=72609
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add what I think was the original premise for an OTB setup:
- setup a database with little to no configuration (no tz customizations) server in same tz as tomcat
- setup a tomcat with little to no configuration (no tz customizations) server in same tz as database
- when persisting org.sakaiproject.time.api.Time / java.util.Date / java.time.Instant (all types with tz info)
- when serializing dates to strings in xml
The base concept was to store these fields in the database as GMT / UTC 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- without this patch
- mysql connector 5.1.49
- tomcat started with -Duser.timezone=Pacific/Honolulu
- mysql> show variables like "%zone%";
+------------------+--------+
| Variable_name | Value |
+------------------+--------+
| system_time_zone | HST |
| time_zone | SYSTEM |
+------------------+--------+
sakai_event table dates are being saved in GMT, whereas with connector 5.1.46, they are saved in HST timezone. This patch (see PR 11967 for the latest patch) attempts to get it to save in HST timezone with the newer connectors.
Are there other configurations that we should include to keep the sakai code out of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"when serializing dates to strings in xml
The base concept was to store these fields in the database as GMT / UTC 0"
What about when storing dates in date fields (not in xml)?
@@ -419,7 +420,7 @@ public boolean transact(Runnable callback, String tag) | |||
} | |||
|
|||
/** Used to work with dates in GMT in the db. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ottenhoff, as the comment states, can you think of anything that requires GMT in the db? Would changing the timezone not be a good idea here? Although it fixed one of my test cases, I wouldn't want to break something else that depends on it being GMT? e.g. does Samigo depend on GMT? I don't know if Samigo would be affected by this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sakai depends on serializing string dates to and from GMT/UTC changing this would likely have consequences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sakai depends on serializing string dates to and from GMT/UTC changing this would likely have consequences.
Please show one place
No description provided.