Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Candidate databse instance: database native time #591

Merged
merged 4 commits into from
Aug 29, 2018

Conversation

shlomi-noach
Copy link
Collaborator

This PR resubmits #587 with only the actual code change. See discussion on #587 and #585

cc @ndelnano

@nickdelnano
Copy link
Contributor

thank you @shlomi-noach !

@nickdelnano
Copy link
Contributor

Is a date for the next release planned? The dates of the last several releases does not look like there is a fixed schedule.

@shlomi-noach
Copy link
Collaborator Author

You are correct, there is no fixed schedule. But a kind reminder helps!

@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster August 27, 2018 05:06 Inactive
@shlomi-noach
Copy link
Collaborator Author

While this may work well for sqlite, this breaks MySQL for servers with non-UTC timezone. Looking into.

@shlomi-noach
Copy link
Collaborator Author

We need to juggle between three different scenarios:

  • MySQL backend (has timezone notion)
  • Sqlite (does not have timezone notion)
  • raft messages (register-candidate can apply by reading historical raft event, the value of now() becomes meaningless)

@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster August 27, 2018 05:46 Inactive
@shlomi-noach
Copy link
Collaborator Author

@ndelnano I ended up solving the problem in a different way: reading the timestamp as a string from the database, then feeding it back.
Would you care to test this PR works for you? Works for me, on sqlite and MySQL on different timezone boxes.

@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster August 27, 2018 05:51 Inactive
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster August 27, 2018 06:13 Inactive
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster August 27, 2018 06:13 Inactive
@shlomi-noach shlomi-noach changed the title Candidate databse instance: time in UTC Candidate databse instance: database native time Aug 27, 2018
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=conductor August 27, 2018 07:37 Inactive
@nickdelnano
Copy link
Contributor

@shlomi-noach I have tested and it does indeed work for me. Thanks again!

@shlomi-noach
Copy link
Collaborator Author

OK good, this has been deployed to staging and in production for a while. I'll let our daily tests run and then merge.

@shlomi-noach shlomi-noach merged commit 769d6cb into master Aug 29, 2018
@shlomi-noach shlomi-noach deleted the candidate-database-instance-utc branch August 29, 2018 05:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants