Skip to content
This repository has been archived by the owner on Jul 27, 2018. It is now read-only.

Do not automatically blow away and re-seed the database in production #403

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jamesatheyDDS
Copy link
Contributor

@jamesatheyDDS jamesatheyDDS commented Mar 30, 2018

This change removes the rails.config file, which currently runs db:setup on every app deploy.

This script is run every time an EC2 instance is spun up as well as every time the app is deployed to an already running instance. In production, it's problematic to blow away the DB and re-seed it every time this happens. Seeding takes multiple minutes, and during that time, various features of the site stop working while they wait for their data to appear in the DB, especially the Locator Maps and the PPM Estimator.

Once this reaches production, changes to the static data in the DB will require either running db:setup manually, or making specific ad-hoc DB changes.

@jamesatheyDDS
Copy link
Contributor Author

FYI, I have already set the EB_ENVIRONMENT variable appropriately in both move-mil-prod and move-mil-staging.

if [ "$EB_ENVIRONMENT" == "move-mil-prod" ]
then
# In production, ensure that DB migrations run because this might be a new version of the app that requires it, but do not blow the DB away and re-seed
su -s /bin/bash -c 'DISABLE_DATABASE_ENVIRONMENT_CHECK=1 bin/rails db:migrate' $(/opt/elasticbeanstalk/bin/get-config container -k app_user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Database migrations are handled in a "pre" hook during deploy. Check out /opt/elasticbeanstalk/hooks/appdeploy/pre/12_db_migration.sh on one of the servers. There might be some useful logic there that could be used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, I don't need to worry about it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. You shouldn't have to run migrations by hand in this script.

@aileendds
Copy link
Contributor

aileendds commented Mar 30, 2018

I'm a bit confused about how this works. Does this mean we should stop using seeds to make changes to the DB? Or does db:migrate seed? How does it know if it should delete an existing row?
Also, I think staging should mirror production. There might be differences between staging and prod that arise because of this change.

@jamesatheyDDS
Copy link
Contributor Author

@aileendds We can continue to use seeds to make changes to the DB, but in production, we should just run bin/rails db:setup manually when we need to apply a change to the database. db:setup is overkill for most changes, but it does get the job done. I'd just like to run it once, when a new version is pushed to production that requires DB changes, instead of every time an instance is spun up.

@jamesatheyDDS jamesatheyDDS force-pushed the no-auto-dbsetup-in-production branch from 1a0cef9 to 63951be Compare April 2, 2018 17:01
@jamesatheyDDS
Copy link
Contributor Author

@jgarber623-gov Since @aileendds pointed out that staging and production should be as close as possible, I am now just removing the rails.config file altogether, since all I think it does is run db:setup. Now, all deployed environments, whether they be staging, production, or per-branch environments (if anyone ever creates another one like I did for the first draft of the PPM estimator) need someone to SSH in and make database changes manually. Thoughts?

@jgarber623-gov
Copy link
Contributor

@jamesatheyDDS If/when you proceed with this change, you'll want to update the developer documentation on the wiki, too. I believe it references the current seeding/deployment process. You'll also want to alert @rich-allen-gov and team so they're aware of this change.

@rich-allen-gov
Copy link
Contributor

I'll have our team follow the issue and try to give them a heads up.

@jamesatheyDDS jamesatheyDDS force-pushed the no-auto-dbsetup-in-production branch from 63951be to b794e65 Compare April 17, 2018 17:12
@jamesatheyDDS
Copy link
Contributor Author

@jgarber623-gov @aileendds @rich-allen-gov I am prepared to alter https://github.com/deptofdefense/move.mil/wiki/Working-with-Seed-Data once this change lands. Is everyone comfortable with proceeding?

@jamesatheyDDS
Copy link
Contributor Author

Also, looking at the Events log on the AWS console, this change will only resolve maybe 1 out of 3 or 1 out of 4 of the "X% of requests are failing with HTTP 5xx" events. Without looking more closely at the logs on the instances, I don't know where the rest of the 5xx errors are coming from.

@jgarber623-gov jgarber623-gov force-pushed the no-auto-dbsetup-in-production branch from b794e65 to 2528c09 Compare July 27, 2018 16:35
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.

4 participants