Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-30186

Implement UUID schema migration

    XMLWordPrintable

    Details

    • Story Points:
      10
    • Sprint:
      DB_S21_12, DB_F21_06
    • Team:
      Data Access and Database
    • Urgent?:
      No

      Description

      With some initial work done on DM-29593 it is time now to switch to specific task of migrating datasets schema to UUID keys. Few items that need some work:

      • we have relatively large number of records in existing repos, tens(s) of millions
      • this needs special care for performance, there are couple of possible options for how to perform this migratrion:
        • dump everything into CSV, run bulk transforms on those CSVs and import it back into database
        • make special mapping table from int dataset_id into UUID and generate new tables by doing "INSERT ... SELECT ... JOIN"
        • latter should be cleaner but there is a question of JOIN performance which I want to study first

        Attachments

          Issue Links

            Activity

            Hide
            salnikov Andy Salnikov added a comment - - edited

            I'll summarize things separately, here is just a quick result of today's migration from the notes that I made:

            • One issue that we encountered immediately is InsufficientPrivilege error. Postgres ALTER TABLE only works for table owner. Chris tried to change schema ownership to me but that did not help, we ended up with me having superuser privileges which of course worked. Possible option for future migrations is that all tables that need ALTER TABLE have to be owned by the same account that runs migration. Because schema has dynamic tables that created by different users it may need superuser intervention to alter ownership for each one of them. Or just do migration from superuser account as we did it today.
            • Second problem was a bug in migration code that failed to pass schema name to one of the migration steps, as a result it tried to change things in default schema (salnikov). The test worked before because I always ran it on a default schema. Lesson here is that tests need to be done on non-default schema name. For the future it would be better to have a special database for such tests where we can recreate few schemas and ownership.
            • Two largest schemas (main and dc2) were migrated in parallel, main finished in 1h10m, similar to my previous tests. dc2 ran in 3h40m, somewhat faster than in my previous tests, I guess without other users we can run things faster.
            Show
            salnikov Andy Salnikov added a comment - - edited I'll summarize things separately, here is just a quick result of today's migration from the notes that I made: One issue that we encountered immediately is InsufficientPrivilege error. Postgres ALTER TABLE only works for table owner. Chris tried to change schema ownership to me but that did not help, we ended up with me having superuser privileges which of course worked. Possible option for future migrations is that all tables that need ALTER TABLE have to be owned by the same account that runs migration. Because schema has dynamic tables that created by different users it may need superuser intervention to alter ownership for each one of them. Or just do migration from superuser account as we did it today. Second problem was a bug in migration code that failed to pass schema name to one of the migration steps, as a result it tried to change things in default schema (salnikov). The test worked before because I always ran it on a default schema. Lesson here is that tests need to be done on non-default schema name. For the future it would be better to have a special database for such tests where we can recreate few schemas and ownership. Two largest schemas (main and dc2) were migrated in parallel, main finished in 1h10m, similar to my previous tests. dc2 ran in 3h40m, somewhat faster than in my previous tests, I guess without other users we can run things faster.
            Hide
            salnikov Andy Salnikov added a comment -

            And here are the metrics from today's migration of production repos (dc2 and main):

            dc2 migration was running from 10:41 to 13:22, main from 10:44 to 11:55 (shaded areas on the plots).

            Show
            salnikov Andy Salnikov added a comment - And here are the metrics from today's migration of production repos (dc2 and main): dc2 migration was running from 10:41 to 13:22, main from 10:44 to 11:55 (shaded areas on the plots).
            Hide
            tjenness Tim Jenness added a comment -

            Andy Salnikov I think we can call this ticket done (at least for postgres).

            Show
            tjenness Tim Jenness added a comment - Andy Salnikov I think we can call this ticket done (at least for postgres).
            Hide
            salnikov Andy Salnikov added a comment -

            I wanted Jim to review it once again but I decided to add a couple of unit tests and small improvements that I did not manage to add before. Should be ready for review tomorrow.

            Show
            salnikov Andy Salnikov added a comment - I wanted Jim to review it once again but I decided to add a couple of unit tests and small improvements that I did not manage to add before. Should be ready for review tomorrow.
            Hide
            salnikov Andy Salnikov added a comment -

            Jim Bosch, I think this is ready for final review. Compared to what you already saw (and what was used for actual migration) I added a bunch of unit tests for most interesting pieces of code and refactored Python code a little bit. I have re-tested it again on my personal repo and it still works OK.

            Most complicated here is probably an actual UUID migration which is in migrations/_oneshot/datasets/int_1.0.0_to_uuid_1.0.0/2101fbf51ad3.py and the code there is somewhat dense (it's a mixture of sqlalchemy and alembic). There is also some duplicated code between this package and daf_butler, not much but eventually I would like to merge that somehow, do not know how yet.

            No rush, it is not blocking anything right now.

            Show
            salnikov Andy Salnikov added a comment - Jim Bosch , I think this is ready for final review. Compared to what you already saw (and what was used for actual migration) I added a bunch of unit tests for most interesting pieces of code and refactored Python code a little bit. I have re-tested it again on my personal repo and it still works OK. Most complicated here is probably an actual UUID migration which is in migrations/_oneshot/datasets/int_1.0.0_to_uuid_1.0.0/2101fbf51ad3.py and the code there is somewhat dense (it's a mixture of sqlalchemy and alembic). There is also some duplicated code between this package and daf_butler, not much but eventually I would like to merge that somehow, do not know how yet. No rush, it is not blocking anything right now.

              People

              Assignee:
              salnikov Andy Salnikov
              Reporter:
              salnikov Andy Salnikov
              Reviewers:
              Jim Bosch
              Watchers:
              Andy Salnikov, Jim Bosch, Kian-Tat Lim, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.