launchpad-reviewers team mailing list archive

[Colin Watson <mp+456017@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:staging-restore-in-place-again into launchpad:master.

Commit message:
Revert "Build staging db inplace due to lack of disk space"

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/456017

This reverts commit 23d8c45e49f6e3338c8d6c10ce1fb55237738700 in principle, with some further changes to work properly in a charmed deployment.  The new staging database servers have disk space for two copies of the database with some left over, so that should be enough to go back to the switching strategy, meaning that staging should no longer need to be down over the weekend.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:staging-restore-in-place-again into launchpad:master.

diff --git a/database/replication/Makefile b/database/replication/Makefile
index 2f69cc0..950c1c4 100644
--- a/database/replication/Makefile
+++ b/database/replication/Makefile
@@ -26,7 +26,6 @@
 ## MULTIPROC=-j 4
 MULTIPROC=
 
-NEW_STAGING_CONFIG=staging-setup # For building the db with a different name.
 STAGING_CONFIG=staging-db # For swapping fresh db into place.
 STAGING_DUMP=launchpad.dump # Dumpfile to build new staging from.
 STAGING_TABLESPACE=pg_default # 'pg_default' for default
@@ -71,24 +70,17 @@ SHHH=
 default:
 	echo Need a target
 
-# Destroy the existing staging db and reload it from dump. We no longer
-# have enough disk space to build it in a holding area and swap it into
-# place.
+# Build _new staging databases from a production dump.
 stagingsetup: DUMPLIST:=$(shell tempfile --suffix=.lst --prefix=lpstag)
 stagingsetup:
-	# List the dump first to make sure it is usable before destroying
-	# anything.
+	# List the dump first to make sure it is usable.
 	pg_restore --list ${STAGING_DUMP} | \
 	    grep -v -E 'TRIGGER public [^ ]+ _sl_' > ${DUMPLIST}
 
-	# Deny new connections to the main DBs and kill any leftovers.
-	set -e; for verb in DISABLE KILL RESUME; do \
-		for db in ${STAGING_PGBOUNCER_MAIN}; do \
-			${STAGING_PGBOUNCER} -c "$$verb $$db"; \
-		done; \
-	done
+	# Kill the existing temporary staging database if it exists from an
+	# aborted run.
 	-LPCONFIG=${STAGING_CONFIG} LP_DESTROY_REMOTE_DATABASE=yes \
-		${PGMASSACRE} ${STAGING_DBNAME_MAIN}
+		${PGMASSACRE} ${STAGING_DBNAME_MAIN}_new
 
 	# Quickly clear out the session DB. No need to DISABLE here, as
 	# we bring the DB back quickly.
@@ -98,32 +90,44 @@ stagingsetup:
 
 	# Create the DB with the desired default tablespace.
 	${CREATEDB} ${STAGING_DBOPTS} --tablespace ${STAGING_TABLESPACE} \
-		${STAGING_DBNAME_MAIN}
+		${STAGING_DBNAME_MAIN}_new
 	# Restore the database. We need to restore permissions, despite
 	# later running security.py, to pull in permissions granted on
 	# production to users not maintained by security.py.
 	< ${STAGING_DUMP} ${STAGING_WALBLOCK} \
-	    | pg_restore ${STAGING_DBOPTS} --dbname=${STAGING_DBNAME_MAIN} \
+	    | pg_restore ${STAGING_DBOPTS} --dbname=${STAGING_DBNAME_MAIN}_new \
 		--no-owner ${EXIT_ON_ERROR} \
 		--use-list=${DUMPLIST} -v
 	rm ${DUMPLIST}
 	# Apply database patches.
 	@echo Running upgrade.py `date`.
 	LPCONFIG=${STAGING_CONFIG} ${SHHH} ../schema/upgrade.py \
-	    --log-file=INFO:${STAGING_LOGDIR}/dbupgrade.log
+		--dbname=${STAGING_DBNAME_MAIN}_new \
+		--log-file=INFO:${STAGING_LOGDIR}/dbupgrade.log
 	@echo Running security.py `date`
 	LPCONFIG=${STAGING_CONFIG} ${SHHH} ../schema/security.py \
-	    --log-file=INFO:${STAGING_LOGDIR}/dbupgrade.log
+		--dbname=${STAGING_DBNAME_MAIN}_new \
+		--log-file=INFO:${STAGING_LOGDIR}/dbupgrade.log
 	@echo Setting feature flags
-	psql ${STAGING_DBOPTS} -d ${STAGING_DBNAME_MAIN} -c "INSERT INTO featureflag (flag, scope, priority, value) VALUES ('profiling.enabled', 'team:launchpad', 0, 'on') ON CONFLICT DO NOTHING"
-	psql ${STAGING_DBOPTS} -d ${STAGING_DBNAME_MAIN} -c "INSERT INTO featureflag (flag, scope, priority, value) VALUES ('librarian.swift.enabled', 'default', 0, 'on') ON CONFLICT DO NOTHING"
+	psql ${STAGING_DBOPTS} -d ${STAGING_DBNAME_MAIN}_new -c "INSERT INTO featureflag (flag, scope, priority, value) VALUES ('profiling.enabled', 'team:launchpad', 0, 'on') ON CONFLICT DO NOTHING"
+	psql ${STAGING_DBOPTS} -d ${STAGING_DBNAME_MAIN}_new -c "INSERT INTO featureflag (flag, scope, priority, value) VALUES ('librarian.swift.enabled', 'default', 0, 'on') ON CONFLICT DO NOTHING"
+
+stagingswitch:
+	set -e; for verb in DISABLE KILL RESUME; do \
+		for db in ${STAGING_PGBOUNCER_MAIN}; do \
+			${STAGING_PGBOUNCER} -c "$$verb $$db"; \
+		done; \
+	done
+	# Kill the existing staging database if it exists.
+	-LPCONFIG=${STAGING_CONFIG} LP_DESTROY_REMOTE_DATABASE=yes \
+		${PGMASSACRE} lpmain_staging
+	# Rename the newly-built staging databases.
+	psql ${STAGING_DBOPTS} -d template1 -c \
+	    "ALTER DATABASE ${STAGING_DBNAME_MAIN}_new RENAME TO ${STAGING_DBNAME_MAIN};"
 	set -e; for db in ${STAGING_PGBOUNCER_MAIN}; do \
 		${STAGING_PGBOUNCER} -c "ENABLE $$db"; \
 	done
 
-stagingswitch:
-	echo Nothing to do. Staging already built inplace.
-
 dogfood: DUMPLIST:=$(shell tempfile --suffix=.lst --prefix=lpdogf)
 dogfood:
 	${CREATEDB} ${DOGFOOD_DBNAME}
diff --git a/database/schema/full-update.py b/database/schema/full-update.py
index fe32fb5..33471cf 100755
--- a/database/schema/full-update.py
+++ b/database/schema/full-update.py
@@ -39,6 +39,7 @@ def run_upgrade(options, log, primary_con):
     options.partial = False
     options.comments = False  # Saves about 1s. Apply comments manually.
     options.separate_sessions = False
+    options.dbname = None
     # Invoke the database schema upgrade process.
     try:
         return upgrade.main(primary_con)
@@ -59,6 +60,7 @@ def run_security(options, log, primary_con):
     options.dryrun = False
     options.revoke = True
     options.owner = "postgres"
+    options.dbname = None
     security.options = options
     security.log = log
     # Invoke the database security reset process.
diff --git a/database/schema/security.py b/database/schema/security.py
index bf13886..0f2af6e 100755
--- a/database/schema/security.py
+++ b/database/schema/security.py
@@ -271,7 +271,7 @@ def main(options, primary_con=None):
     config.read([configfile_name])
 
     if primary_con is None:
-        primary_con = connect()
+        primary_con = connect(dbname=options.dbname)
 
     log.info("Resetting permissions.")
     reset_permissions(primary_con, config, options)
@@ -760,6 +760,16 @@ if __name__ == "__main__":
         default="postgres",
         help="Owner of PostgreSQL objects",
     )
+    parser.add_option(
+        "--dbname",
+        dest="dbname",
+        default=None,
+        metavar="DBNAME",
+        help=(
+            "Connect to DBNAME instead of the database name configured in "
+            "LPCONFIG."
+        ),
+    )
     db_options(parser)
     logger_options(parser)
 
diff --git a/database/schema/upgrade.py b/database/schema/upgrade.py
index ee21fb1..d42369a 100755
--- a/database/schema/upgrade.py
+++ b/database/schema/upgrade.py
@@ -22,15 +22,15 @@ from lp.services.scripts import db_options, logger, logger_options
 SCHEMA_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
-def main(con=None):
+def main(con=None, dbname=None):
     if con is None:
-        con = connect()
+        con = connect(dbname=dbname)
 
     patches = get_patchlist(con)
 
     log.info("Applying patches.")
     if options.separate_sessions:
-        apply_patches_separately()
+        apply_patches_separately(dbname=dbname)
         con.rollback()
     else:
         apply_patches_normal(con)
@@ -173,17 +173,17 @@ def apply_patches_normal(con):
     apply_comments(con)
 
 
-def apply_patches_separately():
+def apply_patches_separately(dbname=None):
     """Update a database, applying each patch in a separate session."""
     # Apply the patches.
-    with connect() as con:
+    with connect(dbname=dbname) as con:
         patches = get_patchlist(con)
     for (major, minor, patch), patch_file in patches:
-        with connect() as con:
+        with connect(dbname=dbname) as con:
             apply_patch(con, major, minor, patch, patch_file)
 
     # Update comments.
-    with connect() as con:
+    with connect(dbname=dbname) as con:
         apply_comments(con)
 
     # Commit changes.  (Not optional, since we don't currently support using
@@ -337,6 +337,16 @@ if __name__ == "__main__":
         action="store_true",
         help="Apply each patch in a separate session",
     )
+    parser.add_option(
+        "--dbname",
+        dest="dbname",
+        default=None,
+        metavar="DBNAME",
+        help=(
+            "Connect to DBNAME instead of the database name configured in "
+            "LPCONFIG."
+        ),
+    )
     (options, args) = parser.parse_args()
 
     if args:
@@ -345,4 +355,4 @@ if __name__ == "__main__":
         parser.error("--dry-run and --separate-sessions are incompatible")
 
     log = logger(options)
-    main()
+    main(dbname=options.dbname)