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

Date Migrations for SQLite & PG when using UTC #7351

Closed
ErisDS opened this Issue Sep 9, 2016 · 24 comments

Comments

Projects
None yet
5 participants
@ErisDS
Member

ErisDS commented Sep 9, 2016

We have had a couple of bugs reported which are in fact a symptom of an issue with the Date migrations we did in Ghost 0.9.

We have identified a logic mistake which means that dates are not in the right format in sqlite3 and postgres if your server was running in UTC when the migration ran.

This mistake will be corrected with a 008 migration, and shipped as 0.11.0.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 14, 2016

Member

Closed by #7323 (774a662)

Member

ErisDS commented Sep 14, 2016

Closed by #7323 (774a662)

@ErisDS ErisDS closed this Sep 14, 2016

@vegasbrianc

This comment has been minimized.

Show comment
Hide comment
@vegasbrianc

vegasbrianc Sep 19, 2016

Hi @ErisDS This still seems to be present in 0.11. I updated 2 old posts and now they are both top of the home page.

vegasbrianc commented Sep 19, 2016

Hi @ErisDS This still seems to be present in 0.11. I updated 2 old posts and now they are both top of the home page.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 19, 2016

Member

@vegasbrianc Can you please config which database version are you using, what version you migrated from, and if you can, please look in your DB and report to us the format that your dates are in?

Member

ErisDS commented Sep 19, 2016

@vegasbrianc Can you please config which database version are you using, what version you migrated from, and if you can, please look in your DB and report to us the format that your dates are in?

@vegasbrianc

This comment has been minimized.

Show comment
Hide comment
@vegasbrianc

vegasbrianc Sep 19, 2016

@ErisDS I'm using the official Docker image and I migrated from .10.1 to 0.11

0.10.1 & 0.11 sql3lite versions are 3.1.4 which were pulled from the package.json file.
Timestamp is a Unixtime stamp UTC - 1465919835431

Let me know if you need anything else.

vegasbrianc commented Sep 19, 2016

@ErisDS I'm using the official Docker image and I migrated from .10.1 to 0.11

0.10.1 & 0.11 sql3lite versions are 3.1.4 which were pulled from the package.json file.
Timestamp is a Unixtime stamp UTC - 1465919835431

Let me know if you need anything else.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 19, 2016

Member

What did Ghost output when you migrated to 0.11? Your timestamps are in the wrong format, but the 0.11 migration should have detected that and fixed it.

Member

ErisDS commented Sep 19, 2016

What did Ghost output when you migrated to 0.11? Your timestamps are in the wrong format, but the 0.11 migration should have detected that and fixed it.

@vegasbrianc

This comment has been minimized.

Show comment
Hide comment
@vegasbrianc

vegasbrianc Sep 19, 2016

I saw the output but didn't notice anything out of the ordinary. Unfortunately, I didn't save it. I will try to restore the DB and rerun the upgrade tomorrow and let you know.

vegasbrianc commented Sep 19, 2016

I saw the output but didn't notice anything out of the ordinary. Unfortunately, I didn't save it. I will try to restore the DB and rerun the upgrade tomorrow and let you know.

@BlackGlory

This comment has been minimized.

Show comment
Hide comment
@BlackGlory

BlackGlory Sep 25, 2016

Same issue, 0.11 still mix UNIX timestamp and human readable date, I upgrade Ghost by Docker too.

BlackGlory commented Sep 25, 2016

Same issue, 0.11 still mix UNIX timestamp and human readable date, I upgrade Ghost by Docker too.

@BlackGlory

This comment has been minimized.

Show comment
Hide comment
@BlackGlory

BlackGlory Sep 25, 2016

I write a script to do date migration, it works for me, maybe helpful.

BlackGlory commented Sep 25, 2016

I write a script to do date migration, it works for me, maybe helpful.

@kirrg001

This comment has been minimized.

Show comment
Hide comment
@kirrg001

kirrg001 Sep 26, 2016

Contributor

@BlackGlory Hey!What did Ghost output when you migrated to 0.11?

Contributor

kirrg001 commented Sep 26, 2016

@BlackGlory Hey!What did Ghost output when you migrated to 0.11?

@vegasbrianc

This comment has been minimized.

Show comment
Hide comment
@vegasbrianc

vegasbrianc Sep 26, 2016

My DB was already overwritten with the 0.11 and I no longer have the previous versions for testing.

vegasbrianc commented Sep 26, 2016

My DB was already overwritten with the 0.11 and I no longer have the previous versions for testing.

@ScottHelme

This comment has been minimized.

Show comment
Hide comment
@ScottHelme

ScottHelme Sep 26, 2016

@BlackGlory I'm having some problems running the script you linked. Any further details?

ScottHelme commented Sep 26, 2016

@BlackGlory I'm having some problems running the script you linked. Any further details?

@ScottHelme

This comment has been minimized.

Show comment
Hide comment
@ScottHelme

ScottHelme Sep 26, 2016

I had this issue on 0.10.0 and updated to 0.11.0 which didn't fix the problem. See here for an example:

https://scotthelme.co.uk/page/4/

ScottHelme commented Sep 26, 2016

I had this issue on 0.10.0 and updated to 0.11.0 which didn't fix the problem. See here for an example:

https://scotthelme.co.uk/page/4/

@BlackGlory

This comment has been minimized.

Show comment
Hide comment
@BlackGlory

BlackGlory Sep 27, 2016

@ScottHelme, You need to export your data from /ghost/settings/labs/ first, then transform, and import the transformed JSON file.

Since #4685 has not been implemented (2 years), you have to delete all content before importing the data, while try ignore the result of all post id growth (backup your ghost.db first).

BlackGlory commented Sep 27, 2016

@ScottHelme, You need to export your data from /ghost/settings/labs/ first, then transform, and import the transformed JSON file.

Since #4685 has not been implemented (2 years), you have to delete all content before importing the data, while try ignore the result of all post id growth (backup your ghost.db first).

@ScottHelme

This comment has been minimized.

Show comment
Hide comment
@ScottHelme

ScottHelme Sep 27, 2016

So things like Disqus Comments that use the post ID will break as a result?

Fortunately I got some help from @kirrg001 who walked me through how to migrate the db file and I'm all fixed now.

ScottHelme commented Sep 27, 2016

So things like Disqus Comments that use the post ID will break as a result?

Fortunately I got some help from @kirrg001 who walked me through how to migrate the db file and I'm all fixed now.

@kirrg001

This comment has been minimized.

Show comment
Hide comment
@kirrg001

kirrg001 Sep 27, 2016

Contributor

I have a solution, which i've tested together with @ScottHelme .

  1. make a backup of your database (!!!!!!!!!!!!!!)
    1.1) make a JSON export and save somewhere
    1.2) copy your sqlite db file somewhere (for example cp content/data/ghost.dev ghost-backup.dev)

  2. login to your database sqlite3 content/data/ghost.db (replace the path when your database file is somewhere else located)

  3. verify you are affected: SELECT created_at from users where id=1 - should show a string format like 2014-08-12 16:53:27

  4. transform the output into an integer using moment

$ cd path-to-your-ghost-folder
$ node
> moment = require('moment-timezone')
> moment.utc('2014-08-12 16:53:27').valueOf()
(replace the date string with your string from (3))
  1. copy the integer value

  2. update created_at to the new value UPDATE users set created_at=your-value where id=1

  3. verify it worked: SELECT created_at from users where id=1

  4. stop your ghost server

  5. run FORCE_MIGRATION=true npm start --production

  6. you should see that the migrations are running, wait until it finishes

  7. kill the process with ctrl+c when finished

  8. start your ghost server as background process (your normal command you use to start ghost)

  9. ensure your blog is alive

  10. if you have trouble with something, replace your sqlite db file with the backup you have created in (1) (cp ghost-backup.dev content/data/ghost.dev)

Write me a direkt message in slack if you have problems!
Good luck and sorry for having trouble!

Contributor

kirrg001 commented Sep 27, 2016

I have a solution, which i've tested together with @ScottHelme .

  1. make a backup of your database (!!!!!!!!!!!!!!)
    1.1) make a JSON export and save somewhere
    1.2) copy your sqlite db file somewhere (for example cp content/data/ghost.dev ghost-backup.dev)

  2. login to your database sqlite3 content/data/ghost.db (replace the path when your database file is somewhere else located)

  3. verify you are affected: SELECT created_at from users where id=1 - should show a string format like 2014-08-12 16:53:27

  4. transform the output into an integer using moment

$ cd path-to-your-ghost-folder
$ node
> moment = require('moment-timezone')
> moment.utc('2014-08-12 16:53:27').valueOf()
(replace the date string with your string from (3))
  1. copy the integer value

  2. update created_at to the new value UPDATE users set created_at=your-value where id=1

  3. verify it worked: SELECT created_at from users where id=1

  4. stop your ghost server

  5. run FORCE_MIGRATION=true npm start --production

  6. you should see that the migrations are running, wait until it finishes

  7. kill the process with ctrl+c when finished

  8. start your ghost server as background process (your normal command you use to start ghost)

  9. ensure your blog is alive

  10. if you have trouble with something, replace your sqlite db file with the backup you have created in (1) (cp ghost-backup.dev content/data/ghost.dev)

Write me a direkt message in slack if you have problems!
Good luck and sorry for having trouble!

@ErisDS ErisDS added the LTS label Sep 29, 2016

@ErisDS ErisDS modified the milestones: 0.11.1, 0.11.0 Sep 29, 2016

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 29, 2016

Member

It's great that we have figured out the cause and a workaround. The question now is, what to do to try to get everyone's data back into a consistent state?

First things first, lets fix the migration for 0.11.1 so that anyone upgrading through won't encounter the problem. I'm not sure what the correct fix is though, ideally we'd only migrate dates that haven't been upgraded. I'm not sure if testing based on the settings.dbHash would be a safer guard, or if we should have stuck with the original check for UTC?

Secondly we handle people who are on 0.11 already. I see a couple of options:

  1. Make it possible to manually force run the date migrations, so people can run something like FORCE_MIGRATION=008 npm start, or DATE_MIGRATE=true npm start and fix their data.
  2. Automatically run the migration again (fixed) for anyone upgrading from 0.11.0 to 0.11.1.
  3. Do a third migration script and bump to 0.12

These all have different up/downsides. The first option is manual, which is just a bit rubbish when we've always done automatic migrations in the past. The second option is a bit of a hack which circumvents our versioning, however it will target only affected people. The third option is possibly the "right way" but also is going to be tricky, I think, to detect what state people are now in and guarantee correctness.

Therefore, I'm leaning towards 2. Fix the migration so that it always works then add an extra line to the bootup to detect anyone that's already on version 008, maybe check if their migration is correct and rerun the migration if not or else (if it's safe to run), run it anyway.

Member

ErisDS commented Sep 29, 2016

It's great that we have figured out the cause and a workaround. The question now is, what to do to try to get everyone's data back into a consistent state?

First things first, lets fix the migration for 0.11.1 so that anyone upgrading through won't encounter the problem. I'm not sure what the correct fix is though, ideally we'd only migrate dates that haven't been upgraded. I'm not sure if testing based on the settings.dbHash would be a safer guard, or if we should have stuck with the original check for UTC?

Secondly we handle people who are on 0.11 already. I see a couple of options:

  1. Make it possible to manually force run the date migrations, so people can run something like FORCE_MIGRATION=008 npm start, or DATE_MIGRATE=true npm start and fix their data.
  2. Automatically run the migration again (fixed) for anyone upgrading from 0.11.0 to 0.11.1.
  3. Do a third migration script and bump to 0.12

These all have different up/downsides. The first option is manual, which is just a bit rubbish when we've always done automatic migrations in the past. The second option is a bit of a hack which circumvents our versioning, however it will target only affected people. The third option is possibly the "right way" but also is going to be tricky, I think, to detect what state people are now in and guarantee correctness.

Therefore, I'm leaning towards 2. Fix the migration so that it always works then add an extra line to the bootup to detect anyone that's already on version 008, maybe check if their migration is correct and rerun the migration if not or else (if it's safe to run), run it anyway.

@ErisDS ErisDS reopened this Sep 29, 2016

@kirrg001

This comment has been minimized.

Show comment
Hide comment
@kirrg001

kirrg001 Sep 29, 2016

Contributor

I would like to suggest something slightly different:
If the original date migration in 006 never ran for a user, then the settings.migrations key should not contain this value. This value is set at the end of the 006 migration.
Even if you have updated to 0.11 and 008 was not successful, it should not exist.

So i will re-write the 008 migration for 0.11.1 to check the values of settings.migrations and if 006 is not existent, we execute 006 again.
For all people who are already on 0.11, i will add a hard code a condition before the migration module boots. This condition looks like: if you are sqlite user and your settings.migrations values do not contain the 006 migration, update your database version to 007.

@ErisDS what do you think?

Contributor

kirrg001 commented Sep 29, 2016

I would like to suggest something slightly different:
If the original date migration in 006 never ran for a user, then the settings.migrations key should not contain this value. This value is set at the end of the 006 migration.
Even if you have updated to 0.11 and 008 was not successful, it should not exist.

So i will re-write the 008 migration for 0.11.1 to check the values of settings.migrations and if 006 is not existent, we execute 006 again.
For all people who are already on 0.11, i will add a hard code a condition before the migration module boots. This condition looks like: if you are sqlite user and your settings.migrations values do not contain the 006 migration, update your database version to 007.

@ErisDS what do you think?

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 29, 2016

Member

So i will re-write the 008 migration for 0.11.1

I assume by this you mean change the existing migration code?

This all sounds like a very good plan to me 👍

Member

ErisDS commented Sep 29, 2016

So i will re-write the 008 migration for 0.11.1

I assume by this you mean change the existing migration code?

This all sounds like a very good plan to me 👍

@kirrg001

This comment has been minimized.

Show comment
Hide comment
@kirrg001

kirrg001 Sep 29, 2016

Contributor

I assume by this you mean change the existing migration code?

Exactly!

Contributor

kirrg001 commented Sep 29, 2016

I assume by this you mean change the existing migration code?

Exactly!

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 29, 2016

Member

Sounds perfect 😁

Member

ErisDS commented Sep 29, 2016

Sounds perfect 😁

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 29, 2016

🐛 optimise 008 date migration fixture
closes TryGhost#7351
- re-write the 008 date migration fixture to ensure 006 gets triggered if it never ran
- adapt tests

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 29, 2016

🐛 optimise 008 date migration fixture
refs TryGhost#7351
- re-write the 008 date migration fixture to ensure 006 gets triggered if it never ran
- adapt tests

@kirrg001 kirrg001 referenced this issue Sep 29, 2016

Merged

🐛 optimise 008 date migration fixture #7459

2 of 2 tasks complete

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 29, 2016

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 30, 2016

🐛 force 008 sqlite user to re-run 008
refs 7351
- see TryGhost#7351 (comment)
- people who still didn't ran the date migration fixture and are on database version 008, will do now with this PR

ErisDS added a commit that referenced this issue Sep 30, 2016

🐛 optimise 008 date migration fixture (#7459)
refs #7351
- re-write the 008 date migration fixture to ensure 006 gets triggered if it never ran
- adapt tests

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 30, 2016

🐛 force 008 sqlite user to re-run 008
refs 7351
- see TryGhost#7351 (comment)
- people who still didn't ran the date migration fixture and are on database version 008, will do now with this PR

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Sep 30, 2016

🐛 force 008 sqlite user to re-run 008
refs 7351
- see TryGhost#7351 (comment)
- people who still didn't ran the date migration fixture and are on database version 008, will do now with this PR

ErisDS added a commit that referenced this issue Sep 30, 2016

🐛 force 008 sqlite user to re-run 008 (#7461)
refs 7351
- see #7351 (comment)
- people who still didn't ran the date migration fixture and are on database version 008, will do now with this PR

ErisDS added a commit to ErisDS/Ghost that referenced this issue Sep 30, 2016

🐛 Fix defaults & default population
refs TryGhost#7351, TryGhost#7461, closes TryGhost#7345

There are kind of two separate fixes here:
- Change the default for migrations to be 008/01 = true, so we can distinguish between fresh installs and upgrades.
- Fix the order of population of default fixtures

ErisDS added a commit to ErisDS/Ghost that referenced this issue Sep 30, 2016

🐛 Fix defaults & default population
refs TryGhost#7351, TryGhost#7461, closes TryGhost#7345

There are kind of two separate fixes here:
- Change the default for migrations to be 008/01 = true, so we can distinguish between fresh installs and upgrades.
- Fix the order of population of default fixtures
@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 30, 2016

Member

Closed by #7465 / 8761579 & #7461 / 3159e6b

Member

ErisDS commented Sep 30, 2016

Closed by #7465 / 8761579 & #7461 / 3159e6b

@ErisDS ErisDS closed this Sep 30, 2016

@ErisDS ErisDS removed the LTS label Oct 7, 2016

yo1dog added a commit to yo1dog/Ghost that referenced this issue Oct 14, 2016

🐛 optimise 008 date migration fixture (TryGhost#7459)
refs TryGhost#7351
- re-write the 008 date migration fixture to ensure 006 gets triggered if it never ran
- adapt tests

yo1dog added a commit to yo1dog/Ghost that referenced this issue Oct 14, 2016

🐛 force 008 sqlite user to re-run 008 (TryGhost#7461)
refs 7351
- see TryGhost#7351 (comment)
- people who still didn't ran the date migration fixture and are on database version 008, will do now with this PR
@vegasbrianc

This comment has been minimized.

Show comment
Hide comment
@vegasbrianc

vegasbrianc Oct 29, 2016

Hi @ErisDS I just upgraded to 0.11.2 and this is now resolved. Thank you very much!

vegasbrianc commented Oct 29, 2016

Hi @ErisDS I just upgraded to 0.11.2 and this is now resolved. Thank you very much!

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Oct 31, 2016

Member

That is really great news. I very much appreciate you taking the time to come back and let us know! Thanks @vegasbrianc 👍

Member

ErisDS commented Oct 31, 2016

That is really great news. I very much appreciate you taking the time to come back and let us know! Thanks @vegasbrianc 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment