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

MIgration of exiting Table to Hypertable not working #13

Closed
daadu opened this issue Apr 17, 2021 · 8 comments · Fixed by #14
Closed

MIgration of exiting Table to Hypertable not working #13

daadu opened this issue Apr 17, 2021 · 8 comments · Fixed by #14

Comments

@daadu
Copy link
Contributor

daadu commented Apr 17, 2021

As described in README:

If you already have a table and want to just add a field you can add the TimescaleDateTimeField to your model. This also triggers the creation of a hypertable.

this is not trigger migration to hypertable.

I digged into the code and found out that "TimescaleSchemaEditor" only overwrites def create_model(self, model):, I don't see how the above mention in "README" would work.

@daadu
Copy link
Contributor Author

daadu commented Apr 17, 2021

Am I doing anything wrong?

@daadu
Copy link
Contributor Author

daadu commented Apr 17, 2021

Also the TimescaleSchemaEditor extends PostGISSchemaEditor instead of django.db.backends.postgresql.schema.DatabaseSchemaEditor.

Should this be fixed?

https://github.com/schlunsen/django-timescaledb/blob/41c7ac2d70d7738c3012389540e1e11fab6391c5/timescale/db/backends/postgresql/schema.py#L1-L6

@daadu
Copy link
Contributor Author

daadu commented Apr 17, 2021

I think we can handle "migration to hypertable" properly (autodetected by the migration framework).

  • For new table - the current implementation works
  • For existing table - there are two cases
    • new "time" field added and use it as partition column
      • migration autodetect feature - will create AddField migration operation
      • which internally calls schema_editor.add_field(from_model, field)
      • we override this method and call super.add_field(from_model, field)
      • then we check if new field is type TimeScaleDateTimeField
      • if that is true we trigger "migrate_to_hypertabe" - procedure for it defined below
    • use "existing datetime" field as partition column
      • migration autdetect feature - will create AlterField migration operation
      • which internally calls schema_editor.alter_field(from_model, from_field, to_field)
      • we override this method and call schema_editor.alter_field(from_model, from_field, to_field)
      • we check if the "type" has changed
      • if that is ture, we trigger "migrate_to_hyertable" procedure

"migrate_to_hypertable" producre

There are two way we can do this:

  1. [FR: migrate existing table with fresh table #16] create new hypertable "like" the old one - insert all rows from old to new; this is what is recommened here
    • this one is recommended as it does not lock the old table, good for large table
    • one issue here is - deleting old(non-hyper)table
      • should this be done automatically - risking data lose? (if implmented correctly wont happen, but cant say!)
      • or we let it be and tell user to "drop" it when he thinks the migration was perfect
  2. create hypertable "with" existing table - with migrate_data => true option when calling create_hypertable; as described here
    • this is relatively simpler to implement and safe (as timescaledb takes the responsibility for "migration")
    • as mentioned in the docs (check below) - this could lock the table and this could be an issue for large tables

      WARNING:The use of the migrate_data argument to convert a non-empty table can lock the table for a significant amount of time, depending on how much data is in the table. It can also run into deadlock if foreign key constraints exist to other tables.

@daadu
Copy link
Contributor Author

daadu commented Apr 17, 2021

@schlunsen let me know your views, so that I can start working on it?

Do we have test framework(I am new to automatic testing)? so that we can check various conditions for migrations ?

@daadu daadu changed the title MIgration to Hypertable not working MIgration of exiting Table to Hypertable not working Apr 17, 2021
@schlunsen
Copy link
Collaborator

Hi @daadu

Thanks for taking the time to look into this, really nice with some movement on this project! I really appreciate that you have documented and highlighted the issues regarding the migration so well.

Would be awesome to have migrations working, which as you correctly observed isn't the case at the moment.

I think both use cases are valid and it could be nice if the we with relatively ease could change the strategy of the migration. Maybe this could be a configurable setting for the migration?

I lean mostly towards having option 2 that you mention as a default and using migrate_data => true when creating the hypertable.

@daadu daadu mentioned this issue Apr 19, 2021
2 tasks
@bencleary
Copy link
Contributor

Hi @daadu,

Really cool input on this! Looks like a really positive change. As for testing I would recommend we use pytest and the standard Django test framework and use a GitHub actions to setup as a CI. I agree with @schlunsen that option 2 is the best option, my use cases are only greenfield, does the migrate_data => true have any impact on new tables or is that ignored by timescale? I can't see anything in the docs.

@daadu
Copy link
Contributor Author

daadu commented Apr 20, 2021

@bencleary for new table (create_model call in schema), the migration passes migrate_data => false, so does not affect that.

@daadu
Copy link
Contributor Author

daadu commented Apr 20, 2021

@bencleary Looks like you have self-assigned #1 , have begin the work for it? where can I check it. would love to add to for migrations.

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 a pull request may close this issue.

3 participants