fixes in response to ekim's comments

INSTALL.md

@@ -45,19 +45,6 @@ After making any changes to `docker-compose.jsonnet`, you will need to rerun `ge
 
 By default the `downloader`, `restreamer`, `backfiller`, `cutter`, `thrimshim` and `nginx` services of the wubloader will be run. To change which services are run edit the `enabled` object in `docker-compose.jsonnet`. A complete wubloader set up also requires one and only one `database` service (though having a backup database is a good idea) and one and only one `sheetsync` service.
 
-### Database setup
-
-When setting up a database node, a number of database specific options can be set.
-
-* `database_path`, the local path to save the database to. If this directory is empty then the database setups scripts will be run to create a new database. Otherwise, the database container will load the database stored in this folder.
-* `db_args.user`, `db_args.password`, the username and password for the database user that the rest of the wubloader will connect to.
-* `db_super_user`, `super_password`, the username and password for the database superuser that is only accessible from the local machine. 
-* `db_replication_user`, `db_replication_password`, the username and password for the database user other nodes can connect as to replicate the database. If `db_replication_user` is an empty string, remote replication will be disabled.
-* `db_standby`, If true this database node will replicate the database node given by `db_args.host`. 
-
-It is recommended that the passwords be changed from the defaults in production.
-A database node needs to expose its database on a port. By default this is `5432` but the port exposed to the outside can be changed in the `ports` object.
-
 ## Running the wubloader
 
 To start the wubloader, simply run
@@ -81,7 +68,7 @@ When setting up a database node, a number of database specific options can be se
 It is recommended that the passwords be changed from the defaults in production.
 A database node needs to expose its database on a port. By default this is `5432` but the port exposed to the outside can be changed in the `ports` object.
 
-The `events` table will be automatically populated by the `sheetsync`. The startup script will attempt to populated the `nodes` and `editors` tables from the `nodes.csv` and `editors.csv` files in `segments_path` directory. The expected format for these files is:
+The `events` table will be automatically populated by the `sheetsync`. If creating a new database, the startup script will attempt to populate the `nodes` and `editors` tables from the `nodes.csv` and `editors.csv` files in `segments_path` directory. The expected format for these files is:
 
 ```
 nodes.csv
@@ -105,3 +92,10 @@ and editors to the database's `editors` table:
 
 `wubloader=> INSERT INTO editors (name, email) VALUES ('example', 'example@gmail.com');`
 
+### Promoting the standby server
+
+To promote the standby server to primary touch the trigger file in the docker container:
+
+`docker exec wubloader_postgres_1 touch /tmp/touch_to_promote_to_master`
+
+Be careful to prevent the original primary from restarting as another primary.

build

@@ -8,7 +8,7 @@ set -eu
 # Pass PUSH=true to also push the resulting images, or PUSH=latest to push them as :latest tag
 
 # The different images we can build
-COMPONENTS=(downloader restreamer backfiller thrimshim cutter sheetsync nginx)
+COMPONENTS=(downloader restreamer backfiller thrimshim cutter sheetsync nginx postgres)
 
 # Define push if not already defined
 PUSH=${PUSH:-}

cutter/cutter/main.py

@@ -502,7 +502,7 @@ def main(dbconnect, youtube_creds_file, name=None, base_dir=".", metrics_port=80
 		except Exception:
 			delay = common.jitter(10)
 			logging.info('Cannot connect to database. Retrying in {:.0f} s'.format(delay))
-			stopping.wait(delay)
+			stop.wait(delay)
 
 	youtube_creds = json.load(open(youtube_creds_file))
 	youtube = Youtube(

db_scripts/db_dump

@@ -1,13 +1,16 @@
 #! /bin/bash
 NAME=${1:-postgres}
+BUCKET=${2:-wubloader-db}
 CONTAINER=$(docker ps --format "{{.Names}}" | grep "$NAME")
-if [ -z "$CONTAINER" ]
-	then
-		echo "Container not found"
-		exit 1
+if [ -z "$CONTAINER" ]; then
+	echo "Container not found"
+	exit 1
 fi
+if [ "$(wc -l <<<"$CONTAINER")" -ne 1 ]; then
+	echo "Multiple containers found"
+	exit 1
+fi
+
 FILE="wubloader-$(date +%Y-%m-%dT%H:%M:%S).sql"
 echo "Dumping $CONTAINER to $FILE"
-docker exec $CONTAINER pg_dump wubloader -U postgres > $FILE
-aws s3 cp $FILE s3://wubloader-test
-rm $FILE
+docker exec $CONTAINER pg_dump wubloader -U postgres | aws s3 cp - "s3://$BUCKET/$FILE"

db_scripts/db_restore

@@ -1,25 +1,31 @@
 #! /bin/bash
 
-if [ -z $1 ]
-	then
-		echo "No SQL script file"
-		exit 1
+if [ -z "$1" ]; then
+	echo "No SQL script file"
+	echo "USAGE: $0 SQL_SCRIPT"
+	exit 1
 fi
 
 NAME=${2:-postgres}
 CONTAINER=$(docker ps --format "{{.Names}}" | grep "$NAME")
-if [ -z "$CONTAINER" ]
-	then
-		echo "Container not found"
-		exit 1
+if [ -z "$CONTAINER" ]; then
+	echo "Container not found"
+	exit 1
+fi
+if [ "$(wc -l <<<"$CONTAINER")" -ne 1 ]; then
+	echo "Multiple containers found"
+	exit 1
 fi
 
 #need to do this in case db dump file has a colon in it
-cp $1 tmp.sql
+cp "$1" tmp.sql
 docker cp tmp.sql "$CONTAINER:/"
 rm tmp.sql
 
-docker exec $CONTAINER dropdb wubloader -U postgres --if-exists
-docker exec $CONTAINER createdb wubloader -U postgres
-docker exec $CONTAINER psql -d wubloader -f dump.sql -U postgres
-docker exec $CONTAINER rm tmp.sql
+# this will fail if there are active sessions by users other than 'postgres'
+# make sure all wubloader components are disconnected
+docker exec "$CONTAINER" dropdb wubloader -U postgres --if-exists
+docker exec "$CONTAINER" createdb wubloader -U postgres
+# this assumes that the vst role is in the postgres database
+docker exec "$CONTAINER" psql -d wubloader -f tmp.sql -U postgres
+docker exec "$CONTAINER" rm tmp.sql

docker-compose.jsonnet

@@ -57,6 +57,7 @@
   backdoor_port:: 1234,
 
   // Other nodes to always backfill from. You should not include the local node.
+  // If you are using the database to find peers, you should leave this empty.
   peers:: [
   ],
 
@@ -64,7 +65,7 @@
   // If database is defined in this config, host and port should be postgres:5432.
   db_args:: {
     user: "vst",
-    password: "dbfh2019", // don't use default in production
+    password: "dbfh2019", // don't use default in production. Must not contain ' or \ as these are not escaped.
     host: "postgres",
     port: 5432,
     dbname: "wubloader",
@@ -72,9 +73,9 @@
 
   // Other database arguments
   db_super_user:: "postgres", // only accessible from localhost
-  db_super_password:: "postgres",
+  db_super_password:: "postgres", // Must not contain ' or \ as these are not escaped.
   db_replication_user:: "replicate", // if empty, don't allow replication
-  db_replication_password:: "standby", // don't use default in production
+  db_replication_password:: "standby", // don't use default in production. Must not contain ' or \ as these are not escaped.
   db_standby:: false, // set to true to have this database replicate another server
 
   // Path to a JSON file containing google credentials as keys

postgres/Dockerfile

@@ -1,5 +1,5 @@
 FROM postgres:latest
 COPY postgres/setup.sh /docker-entrypoint-initdb.d/setup.sh
-COPY postgres/standby_setup.sh /standby_setup.sh
 RUN chmod 0666 /docker-entrypoint-initdb.d/setup.sh
+COPY postgres/standby_setup.sh /standby_setup.sh
 RUN chmod 0700 /standby_setup.sh

postgres/setup.sh

@@ -2,6 +2,7 @@
 
 set -e
 
+# only allow the $WUBLOADER_USER to connect remotely rather than all users
 sed -i "/host all all all/d" "$PGDATA/pg_hba.conf"
 echo "host all $WUBLOADER_USER all md5" >> "$PGDATA/pg_hba.conf"
 
@@ -15,6 +16,7 @@ EOSQL
 
 if [ -n "$REPLICATION_USER" ]; then
 	echo "Creating $REPLICATION_USER"
+	# allow the $REPLICATION user to replicate remotely
 	echo "host replication $REPLICATION_USER all md5" >> "$PGDATA/pg_hba.conf"
 	psql -v ON_ERROR_STOP=1 -U postgres <<-EOSQL
 
@@ -40,7 +42,7 @@ if [ -a /mnt/wubloader/nodes.csv ]; then
 		url TEXT NOT NULL,
 		backfill_from BOOLEAN NOT NULL DEFAULT TRUE);
 	COPY nodes FROM '/mnt/wubloader/nodes.csv' DELIMITER ',' CSV HEADER;
-	ALTER TABLE nodes OWNER TO vst;
+	ALTER TABLE nodes OWNER TO $WUBLOADER_USER;
 	EOF
 fi
 
@@ -51,7 +53,7 @@ if [ -a /mnt/wubloader/editors.csv ]; then
 		email TEXT PRIMARY KEY,
 		name TEXT NOT NULL);
 	COPY editors FROM '/mnt/wubloader/editors.csv' DELIMITER ',' CSV HEADER;
-	ALTER TABLE editors OWNER TO vst;
+	ALTER TABLE editors OWNER TO $WUBLOADER_USER;
 	EOF
 fi
 

postgres/standby_setup.sh

@@ -1,15 +1,17 @@
 #! /bin/bash
 
+set -e
 
+# if postgres database does not exist in $PGDATA
 if [ ! -s "$PGDATA/PG_VERSION" ]; then
 
+	# get a binary backup of the database on $MASTER_NODE
 	pg_basebackup -d "host=$MASTER_NODE password=$REPLICATION_PASSWORD port=5432 user=$REPLICATION_USER" -D ${PGDATA} -vP
-
-	set -e
-
+	
 	cat > ${PGDATA}/recovery.conf <<-EOF
 	standby_mode = on
 	primary_conninfo = 'host=$MASTER_NODE password=$REPLICATION_PASSWORD port=5432 user=$REPLICATION_USER'
+	# touch this file to promote this node to master
 	trigger_file = '/tmp/touch_to_promote_to_master'
 	EOF
 
@@ -18,4 +20,5 @@ if [ ! -s "$PGDATA/PG_VERSION" ]; then
 
 fi
 
+# start postgres 
 gosu postgres postgres

sheetsync/sheetsync/main.py

@@ -281,14 +281,13 @@ def main(dbconnect, sheets_creds_file, edit_url, bustime_start, sheet_id, worksh
 	logging.info("Starting up")
 
 	dbmanager = None
-	stopping = gevent.event.Event()
 	while dbmanager is None:
 		try:
 			dbmanager = DBManager(dsn=dbconnect)
 		except Exception:
 			delay = common.jitter(10)
 			logging.info('Cannot connect to database. Retrying in {:.0f} s'.format(delay))
-			stopping.wait(delay)
+			stop.wait(delay)
 
 	sheets_creds = json.load(open(sheets_creds_file))
 

thrimshim/thrimshim/main.py

@@ -2,6 +2,7 @@ import datetime
 import json
 import logging
 import signal
+import sys
 
 import argh
 import flask
@@ -221,21 +222,27 @@ def main(connection_string, host='0.0.0.0', port=8004, backdoor_port=0):
 	"""Thrimshim service."""
 	server = WSGIServer((host, port), cors(app))
 
-	app.no_authentication = no_authentication
-	app.db_manager = None
 	stopping = gevent.event.Event()
-	while app.db_manager is None:
+	def stop():
+		logging.info("Shutting down")
+		stopping.set()
+		# handle when the server is running
+		if hasattr(server, 'socket'):
+			server.stop()
+		# and when not
+		else:
+			sys.exit()
+
+	gevent.signal(signal.SIGTERM, stop)
+
+	app.db_manager = None
+	while app.db_manager is None and not stopping.is_set():
 		try:
 			app.db_manager = database.DBManager(dsn=connection_string)
 		except Exception:
 			delay = common.jitter(10)
 			logging.info('Cannot connect to database. Retrying in {:.0f} s'.format(delay))
 			stopping.wait(delay)
-	
-	def stop():
-		logging.info("Shutting down")
-		server.stop()
-	gevent.signal(signal.SIGTERM, stop)
 
 	common.PromLogCountsHandler.install()
 	common.install_stacksampler()