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

use UTC in MySQL backend for workflow started timestamp #71

Closed

Conversation

jessepeterson
Copy link
Member

Appropriately handle timestamps for the "workflow started" engine storage interfaces in the MySQL storage backend.

We had a few critical issues with this—namely from using a MySQL TIMESTAMP column and using a normal time.Now() value:

  • TIMESTAMP columns convert to UTC for storage, but always parse and present the MySQL system/session local timezone. From the manual: "MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval."
  • This was especially a problem because we took the time.Time{} (i.e. time.Now() with the local TZ) value as-is and simply converted it to a string and stored it.
  • In a typical scenario this meant that while a timestamp was inserted as the local TZ (and converted for storage by MySQL to UTC) but when MySQL read it back (as the local TZ) we utilized UTC to parse the time. This meant it was always off by the local MySQL TZ offset in Go.
  • Further if the TZ of the MySQL session was different (say, your MySQL server is in a different TZ than your server code) then not only would the above issue exist, but it'd be even more problematic as the local TZ time would be interpreted as the MySQL session TZ and UTC calculated from that for the storage of the timestamp.

To address all that we now explicitly convert the input Go time.Time{} to UTC, then use MySQL CONVERT_TZ() to convert that UTC timestamp to the session TZ (and: yes, just for MySQL to convert it right back to UTC for storage). As well for reading we ask MySQL to convert from the "presented" session TZ back to UTC (again using CONVERT_TZ()) so that Go can use UTC when parsing the queried time. This keeps everything in UTC for the date math in Golang. Updated the tests.

An alternate solve for this was to add another storage interface method that explicitly do the date math in the storage backend. E.g. something like IsWorkflowStartedBefore(time.Duration). That may still be a good choice for the future.

I've been warned against using TIMESTAMP columns in general with MySQL. I was a cavalier with that advice. This is the prize.

@jessepeterson
Copy link
Member Author

Closing in favor of #72.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant