artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 16:11:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

migrations: New migrations use transactions (#2486)

Browse Source 
We discovered that the migration library does not use transactions, nor has it
ever used transactions. The only place it uses transactions is for SetVersion,
not the actual migrations. The [README][1] for the postgres driver encourages
the use of transactions for multi-statement migrations. We believe it is better
to just always use transactions.

We added a template for the created migrations. It uses a transaction and it includes comments with advice from the README.

[1]: https://sourcegraph.com/github.com/golang-migrate/migrate@v4.2.5/-/blob/database/postgres/README.md?view=code#L26
This commit is contained in:
Keegan Carruthers-Smith 2019-03-01 09:42:16 +02:00 committed by GitHub
parent 7d2bfc0c2e
commit 36a4c13db8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 33 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
.github/CODEOWNERS vendored
View File

@ -7,8 +7,6 @@
/web/src/ @felixfbecker
/cmd/frontend/internal/app/pkg/updatecheck/ @dadlerj
/web/src/tracking/ @dadlerj
/cmd/gitserver/ @sourcegraph/core-services
/cmd/repo-updater/ @sourcegraph/core-services
/cmd/frontend/authz/ @beyang
/cmd/frontend/auth/ @beyang
/cmd/frontend/internal/auth/ @beyang
@ -19,6 +17,11 @@
/enterprise/cmd/frontend/internal/authz @beyang
/enterprise/cmd/frontend/auth @beyang
# Core Services
/cmd/gitserver/ @sourcegraph/core-services
/cmd/repo-updater/ @sourcegraph/core-services
/migrations/ @sourcegraph/core-services
# Search
/cmd/frontend/graphqlbackend/*search* @sourcegraph/code-search
*/search/**/* @sourcegraph/code-search

27
dev/add_migration.sh
View File

@ -1,8 +1,29 @@
#!/bin/bash
cd $(dirname "${BASH_SOURCE[0]}")/../migrations
set -e
# The name is intentionally empty ('') so that it forces a merge conflict if two branches attempt to
# create a migration at the same sequence number (because they will both add a file with the same
# name, like `migrations/1528277032_.up.sql`).
unset CDPATH
cd migrations && migrate create -ext sql -dir . -digits 10 -seq ''
echo Empty migration files added in migrations/
migrate create -ext sql -dir . -digits 10 -seq ''
files=$(ls -1 | grep '^[0-9]'.*\.sql | sort -n | tail -n2)
for f in $files; do
cat > $f <<EOF
BEGIN;
-- Insert migration here. See README.md. Highlights:
-- * Always use IF EXISTS. eg: DROP TABLE IF EXISTS global_dep_private;
-- * All migrations must be backward-compatible. Old versions of Sourcegraph
-- need to be able to read/write post migration.
-- * Historically we advised against transactions since we thought the
-- migrate library handled it. However, it does not! /facepalm
COMMIT;
EOF
echo "Created migrations/$f"
done

8
migrations/README.md
View File

@ -21,10 +21,10 @@ Run the following:
./dev/add_migration.sh
```
There will be up/down `.sql` migration files created in this directory. Add SQL statements to these
files that will perform the desired migration. **NOTE**: the migration runner wraps each migration
script in a transaction block; do not add explicit transaction blocks to the migration script as
this has caused issues in the past.
There will be up/down `.sql` migration files created in this directory. Add
SQL statements to these files that will perform the desired
migration. **NOTE**: the migration runner does not use transactions. Use the
explicit transaction blocks added to the migration script template.
```sql
# Enter statements here