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

Merge the Databases #1372

Merged
merged 36 commits into from
Apr 15, 2024
Merged

Merge the Databases #1372

merged 36 commits into from
Apr 15, 2024

Conversation

Mythicaeda
Copy link
Contributor

@Mythicaeda Mythicaeda commented Mar 21, 2024

Description

Breaking Changes:

  • Scheduling Specs are now created and populated automatically. This is breaking for the UI and CLI
  • Envvars have been added and renamed

This PR goes through all of our databases and combine them into a single aerie database. This change involved:

  • Moving and reorganizing our existing SQL files
  • Adding Schema Qualifiers to all DB Objects and DB Object references (this includes in the backend code)
  • Adding missing Foreign Key relationships
  • Updating all of our Hasura metadata to preserve the GQL interface and to replace manual relationships with FKey relationships where possible.
  • Extracting repeated trigger methods that were not schema-specific (ie set new.revision = old.revision +1) into functions in the util_functions schema.
  • Making each service use its own DB user. (This is the reason for the envvar breaking change)
  • Updating and simplifying the Aerie DB Helper Script

Verification

This PR will not pass the E2E Test Workflow. The Gateway code needed to be updated as well, and this change was breaking. The develop Gateway that the e2eTest workflow uses is unable to upload files to this backend.
This PR will not pass the DB Migration Check Workflow. That workflow has been updated to work with future migrations once this PR has been released (note: if this PR is merged post-2.8.0, the Workflow will need to be updated). Instead, the migration was tested as follows:

  1. (Prerequisite) Clone pgcmp. I ran all commands from within the folder it was cloned into.
  2. (Prerequisite) Make sure you have libpq installed with brew install libpq.
  3. Spin up this branch (make sure you've updated you .env).
  4. Create the folders to dump the DBs into: mkdir -p mergeDmp migrationDmp
  5. Dump this version of the DB with the following command:
PGURI=postgres://aerie:aerie@localhost:5432/aerie \
PGCMPOUTPUT=./mergeDmp/AerieMerged \
PGCLABEL=AerieMerged \
PGBINDIR=/opt/homebrew/opt/libpq/bin \
./pgcmp-dump
  1. Destroy the Aerie instance, spin up develop. (I recommend running ./gradlew clean afterwards to get rid of the copied SQL files)
  2. Check out this branch.
  3. Follow the instructions in Documentation to migrate the develop DB to the merged DB
  4. Dump the migrated DB with the following command:
PGURI=postgres://aerie:aerie@localhost:5432/aerie \
PGCMPOUTPUT=./migrateDmp/AerieMigrated \
PGCLABEL=AerieMigrated \
PGBINDIR=/opt/homebrew/opt/libpq/bin \
./pgcmp-dump
  1. Compare the two dumped DBs with the following script. The expected output ends with: Database comparison succeeded
#!/bin/bash

mkdir results
mkdir comparison

PGCMPINPUT1=./mergeDmp/AerieMerged \
PGCMPINPUT2=./migrateDmp/AerieMigrated \
PGCLABEL1=AerieMerged \
PGCLABEL2=AerieMigrated \
PGCFULLOUTPUT=./comparison/fulloutput.txt \
PGCUNEXPLAINED=./comparison/unexplained.txt \
PGCBADEXPLAIN=./comparison/badexplanations.txt \
PGDB=postgres \
PGPASSWORD=postgres \
PGBINDIR=/opt/homebrew/opt/libpq/bin \
PGCOMITSCHEMAS="('hdb_catalog'),('pg_catalog'),('information_schema')" \
./pgcmp
return_code=$?
if [ $return_code -ne 0 ]; then
    echo "Database comparison failed - return code=$return_code"
    mv ./comparison/fulloutput.txt results/fulloutput
    mv ./comparison/unexplained.txt results/unexplained
    mv ./comparison/badexplanations.txt results/badexplanations
    mv /tmp/perform-comparison.log results/perform-comparison.log
else
    echo "Database comparison succeeded"
fi

exit $return_code

This process was run both on a "raw" database and on one that had simulation datasets on it, since datasets are the only part of the DB using partition tables and execute statements.

Other Test changes:

  • Aside from making the DB Comparison Workflow work again, this PR additionally has it officially exclude the HDB_Catalog schema. This schema was already effectively excluded via our "expected mismatch" file, but testing revealed that this schema is officially controlled exclusively via Hasura, meaning we shouldn't be migrating it or concerned with changes Hasura makes to it.
  • E2E Tests no longer attempt to insert a scheduling spec
  • DB Tests had several updates. See the commit body for move details.
    • Schema info and language tags were added to SQL statements
    • Suppressed SqlSourceToSinkFlow warnings, which are not significant since this is test code
    • Extracted duplicate SQL code into methods and moved some methods to the MerlinDBTestHelper class
    • DBTestHelper now closes the connection and datasource when shutting down, no longer needs init File passed it on startup, and has gained a helper method clearSchema

Documentation

The migration script guide in our docs needs to be updated.

How to run this migration needs to be included in our upgrade docs:

Prerequisites:

  • If you are not using the default names for the Aerie databases (aerie_merlin, aerie_scheduler, aerie_sequencing, aerie_ui), you must rename them to the default names.
  • Your set of database migrations must include all migrations released as of v2.6.0
  • Update your .env and docker-compose.yaml using the provided files in deployment. Specifically, the following envvars have been added to the .env:
# Credentials for the Gateway Database User
GATEWAY_USERNAME= 
GATEWAY_PASSWORD=
# Credentials for the Merlin Database User
MERLIN_USERNAME=
MERLIN_PASSWORD=
# Credentials for the Scheduling Database User
SCHEDULER_USERNAME=
SCHEDULER_PASSWORD=
# Credentials for the Sequencing Database User
SEQUENCING_USERNAME=
SEQUENCING_PASSWORD=

and the containers enviornment variables have been updated as follows:

  • aerie_gateway:
    • removed envvars:
      • POSTGRES_AERIE_MERLIN_DB
      • POSTGRES_USER
      • POSTGRES_PASSWORD
    • renamed envvars:
      • POSTGRES_HOST -> AERIE_DB_HOST
      • POSTGRES_PORT -> AERIE_DB_PORT
    • added envvars:
      • GATEWAY_DB_USER: "${GATEWAY_USERNAME}"
      • GATEWAY_DB_PASSWORD: "${GATEWAY_PASSWORD}"
  • aerie_merlin:
    • removed envvars: MERLIN_DB
    • renamed envvars:
      • MERLIN_DB_SERVER -> AERIE_DB_HOST
      • MERLIN_DB_PORT -> AERIE_DB_PORT
    • MERLIN_DB_USER should now be mapped at the envvar "${MERLIN_USERNAME}"
    • MERLIN_DB_PASSWORD should now be mapped at the envvar "${MERLIN_PASSWORD}"
  • aerie_merlin_workers: (for each worker)
    • removed envvars:
      • MERLIN_WORKER_DB
      • MERLIN_WORKER_DB_USER
      • MERLIN_WORKER_DB_PASSWORD
    • renamed envvars:
      • MERLIN_WORKER_DB_SERVER -> AERIE_DB_HOST
      • MERLIN_WORKER_DB_PORT -> AERIE_DB_PORT
    • added envvars:
      • MERLIN_DB_USER: "${MERLIN_USERNAME}"
      • MERLIN_DB_PASSWORD: "${MERLIN_PASSWORD}"
  • aerie_scheduler:
    • removed envvars: SCHEDULER_DB
    • renamed envvars:
      • SCHEDULER_DB_SERVER -> AERIE_DB_HOST
      • SCHEDULER_DB_PORT -> AERIE_DB_PORT
    • SCHEDULER_DB_USER should now be mapped at the envvar "${SCHEDULER_USERNAME}"
    • SCHEDULER_DB_PASSWORD should now be mapped at the envvar "${SCHEDULER_PASSWORD}"
  • aerie_scheduler_workers: (for each worker)
    • removed envvars:
      • SCHEDULER_WORKER_DB
      • SCHEDULER_WORKER_DB_USER
      • SCHEDULER_WORKER_DB_PASSWORD
    • renamed envvars:
      • SCHEDULER_WORKER_DB_SERVER -> AERIE_DB_HOST
      • SCHEDULER_WORKER_DB_PORT -> AERIE_DB_PORT
    • added envvars:
      • SCHEDULER_DB_USER: "${SCHEDULER_USERNAME}"
      • SCHEDULER_DB_PASSWORD: "${SCHEDULER_PASSWORD}"
  • aerie_sequencing:
    • removed envvars: SEQUENCING_DB
    • renamed envvars:
      • SEQUENCING_DB_SERVER -> AERIE_DB_HOST
      • SEQUENCING_DB_PORT -> AERIE_DB_PORT
    • SEQUENCING_DB_USER should now be mapped at the envvar "${SEQUENCING_USERNAME}"
    • SEQUENCING_DB_PASSWORD should now be mapped at the envvar "${SEQUENCING_PASSWORD}"
  • hasura:
    • removed envvars:
      • AERIE_MERLIN_DATABASE_URL
      • AERIE_SCHEDULER_DATABASE_URL
      • AERIE_SEQUENCING_DATABASE_URL
      • AERIE_UI_DATABASE_URL
    • added envvars:
      • AERIE_DATABASE_URL: "postgres://${AERIE_USERNAME}:${AERIE_PASSWORD}@postgres:5432/aerie?options=-c%20search_path%3Dutil_functions%2Chasura%2Cpermissions%2Ctags%2Cmerlin%2Cscheduler%2Csequencing%2Cpublic"
  • postgres:
    • added envvars:
      • GATEWAY_DB_USER: "${GATEWAY_USERNAME}"
      • GATEWAY_DB_PASSWORD: "${GATEWAY_PASSWORD}"
      • MERLIN_DB_USER: "${MERLIN_USERNAME}"
      • MERLIN_DB_PASSWORD: "${MERLIN_PASSWORD}"
      • SCHEDULER_DB_USER: "${SCHEDULER_USERNAME}"
      • SCHEDULER_DB_PASSWORD: "${SCHEDULER_PASSWORD}"
      • SEQUENCING_DB_USER: "${SEQUENCING_USERNAME}"
      • SEQUENCING_DB_PASSWORD: "${SEQUENCING_PASSWORD}"

Migration:

  1. From within the merge_aerie_db directory, execute merge_db.sh. Usage is below. If you would like to drop the old DBs as part of the task, ensure the non-postgres containers are not running.
Migrate from a pre-v2.8.0 Aerie Database to v2.8.0

usage: merge_db.sh [-h] [-d] [-e ENV_PATH] [-p HASURA_PATH] [-n NETWORK_LOCATION]
options:
-h    print this message and exit
-d    drop the old databases after the merge
-e    path to the .env file used to deploy v2.8.0 Aerie. defaults to ../.env
-p    path to the directory containing the config.yaml for the aerie deployment. defaults to ../hasura
-n    network location of the database. defaults to localhost
  1. Start up the rest of the Aerie Services

If you encounter any errors while running merge_db.sh, it is safe drop the aerie database and restart the migration. If you ran the migration with the -d flag and encountered the error during the Dropping old Databases command, however, the migration was successful and that step can be reattempted by itself using the drop_old_dbs.sh script. See below for usage.

If you did not run the migration step with the -d flag, you can drop the old databases using the drop_old_dbs.sh script. Usage is below:

Drop the pre-v2.8.0 unmerged Aerie Databases

usage: drop_old_dbs.sh [-h] [-e ENV_PATH] [-n NETWORK_LOCATION]
options:
-h    print this message and exit
-e    path to the .env file used to deploy v2.8.0 Aerie. defaults to ../.env
-n    network location of the database. defaults to localhost

Future work

  • Update Sequencing and Scheduler to no longer use Hasura connections to query Merlin. This should definitely be done for Sequencing, but the Scheduler would need a performance analysis to see if this would be more performant than downloading/uploading sim results in multiple parallel GQL Queries
  • Activity Directive is the only table to use “last_modified_at” instead of “updated_at”. It should be updated to be consistent with the rest of the DB.
  • Update Postgres to v16
  • The non-merlin schemas can now have plan-owner-based permissions written for them
  • The DB Tests can be refactored better. IE, there is set up code that is no longer in the merlin-exclusive part of the DB that can be pulled out of the MerlinDBHelper.
  • A pass needs to be done over the functions and reevalute the security definer vs security invoker permissions.

@Mythicaeda Mythicaeda added breaking change A change that will require updating downstream code refactor A code change that neither fixes a bug nor adds a feature database Anything related to the database labels Mar 21, 2024
@Mythicaeda Mythicaeda requested a review from skovati March 21, 2024 17:47
@Mythicaeda Mythicaeda self-assigned this Mar 21, 2024
@Mythicaeda Mythicaeda requested a review from a team as a code owner March 21, 2024 17:47
@Mythicaeda Mythicaeda requested a review from cohansen March 21, 2024 17:47
@Mythicaeda Mythicaeda force-pushed the refactor/merge-the-dbs branch from f21e1a8 to baf77c7 Compare March 21, 2024 21:23
@dandelany
Copy link
Collaborator

Thanks @Mythicaeda - the instructions above note that you need to "Update your .env" before testing. Can you write up a bit more detail on which variables have been renamed/added that need to be populated? I can probably figure it out from the code but we'll want to add this to the upgrade guide when we release.

@Mythicaeda
Copy link
Contributor Author

Thanks @Mythicaeda - the instructions above note that you need to "Update your .env" before testing. Can you write up a bit more detail on which variables have been renamed/added that need to be populated? I can probably figure it out from the code but we'll want to add this to the upgrade guide when we release.

I've updated the PR Description. Let me know if there's any areas that need more clarification!

- Update databases.yaml
- Reorder contents of schema-level tables.yamls to better resemble schema-level init.sqls
- Track `preset_to_snapshot_directive`
- Translate manual configurations to use fkeys where possible
- Update remote relationships to non-remote relationships
- Fixed array relationship `expansion_sets` on `expansion_set_to_rule` that should be an object relationship (BREAKING CHANGE)
Adds fkeys that were previously unexpressable due to the tables being in separate DBs

Adds a trigger to create a Scheduling Spec when a plan is created (this is breaking for the UI)
Adds fkeys that were previously unexpressable due to the tables being in separate DBs
Adds fkeys that were previously unexpressable due to the tables being in separate DBs
Additionally refines the comment on `expansion_rule.activity_type`
- Update `deallocate` to remove unneeded reference to the Spec ID and to properly pass analysisID instead of spec revision
…st_status

[Hasura is currently unable to properly use custom types not in the public schema] (hasura/graphql-engine#4630). While this can be partially circumvented by specifying a search_path in the connection string (see change to Docker Compose), because these two types share a name, Hasura would always match to the type specified in the first listed schema in the search path. Because these types are identical sans the originating DB, they were combined into a singular type, `request_status`, rather than renaming each one.
- Add schema info and language tags to SQL statements
- Suppress `SqlSourceToSinkFlow` warnings, which are not significant since this is test code
- Extract duplicate SQL code into methods

- Close the connection and datasource when stopping DB in `DatabaseTestHelper`. Method renamed to `close` to reflect this.
- Create helper method `DBTestHelper::clearSchema` to simplify test cleanup code

- Move `PlanCollaborationTests::UpdateActivityName` and `DeleteActivityDirective` to `MerlinDBTestHelper`
- Wrap created statements in "try-with-resources"
- Fix Plan Collaboration Test `mergeBaseBetweenSelf`, which wasn't calling `get_merge_base` before
- Replace inserting a scheduling spec with fetching the automatically generated one's id
- Remove outdated explanations
- Take v2.8.0 as base
- Exclude the HDB_Catalog schema from throwing comparison failures, as this schema is controlled by Hasura
Done in case the database has any rows that would violate the foreign key about to be added.
@Mythicaeda Mythicaeda force-pushed the refactor/merge-the-dbs branch from cf71ef0 to 3a285c9 Compare April 15, 2024 22:21
@Mythicaeda Mythicaeda merged commit 19ceee2 into develop Apr 15, 2024
8 of 10 checks passed
@Mythicaeda Mythicaeda deleted the refactor/merge-the-dbs branch April 15, 2024 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change A change that will require updating downstream code database Anything related to the database refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge the Aerie Databases
4 participants