← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:remove-activity-cols into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:remove-activity-cols into launchpad:master.

Commit message:
Remove lp.services.database.activity_cols

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This was used to handle some changes in PostgreSQL 9.2 (https://www.postgresql.org/docs/9.2/release-9-2.html#AEN114556).  Now that we require at least PostgreSQL 10, it just makes the relevant queries harder to read.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-activity-cols into launchpad:master.
diff --git a/database/schema/preflight.py b/database/schema/preflight.py
index e05e1c8..5ac9309 100755
--- a/database/schema/preflight.py
+++ b/database/schema/preflight.py
@@ -22,7 +22,6 @@ import psycopg2
 
 import upgrade
 from dbcontroller import DBController, streaming_sync
-from lp.services.database import activity_cols
 from lp.services.database.sqlbase import ISOLATION_LEVEL_AUTOCOMMIT, sqlvalues
 from lp.services.scripts import logger, logger_options
 from replication.helpers import Node
@@ -142,10 +141,9 @@ class DatabasePreflight:
                 FROM pg_stat_activity
                 WHERE
                     datname=current_database()
-                    AND %(pid)s <> pg_backend_pid()
+                    AND pid <> pg_backend_pid()
                 GROUP BY datname, usename
                 """
-                % activity_cols(cur)
             )
             for datname, usename, num_connections in cur.fetchall():
                 if usename in SYSTEM_USERS:
@@ -177,18 +175,15 @@ class DatabasePreflight:
         for node in self.lpmain_nodes:
             cur = node.con.cursor()
             cur.execute(
-                (
-                    """
+                """
                 SELECT datname, usename, COUNT(*) AS num_connections
                 FROM pg_stat_activity
                 WHERE
                     datname=current_database()
-                    AND %(pid)s <> pg_backend_pid()
-                    AND usename IN %%s
+                    AND pid <> pg_backend_pid()
+                    AND usename IN %s
                 GROUP BY datname, usename
                 """
-                    % activity_cols(cur)
-                )
                 % sqlvalues(FRAGILE_USERS)
             )
             for datname, usename, num_connections in cur.fetchall():
@@ -376,19 +371,14 @@ class KillConnectionsPreflight(DatabasePreflight):
             for node in nodes:
                 cur = node.con.cursor()
                 cur.execute(
-                    (
-                        """
-                    SELECT
-                        %(pid)s, datname, usename,
-                        pg_terminate_backend(%(pid)s)
+                    """
+                    SELECT pid, datname, usename, pg_terminate_backend(pid)
                     FROM pg_stat_activity
                     WHERE
                         datname=current_database()
-                        AND %(pid)s <> pg_backend_pid()
-                        AND usename NOT IN %%s
+                        AND pid <> pg_backend_pid()
+                        AND usename NOT IN %s
                     """
-                        % activity_cols(cur)
-                    )
                     % sqlvalues(SYSTEM_USERS)
                 )
                 for pid, datname, usename, ignored in cur.fetchall():
diff --git a/database/schema/unautovacuumable.py b/database/schema/unautovacuumable.py
index 69ab428..d888726 100755
--- a/database/schema/unautovacuumable.py
+++ b/database/schema/unautovacuumable.py
@@ -21,7 +21,6 @@ import sys
 import time
 from optparse import OptionParser
 
-from lp.services.database import activity_cols
 from lp.services.database.sqlbase import ISOLATION_LEVEL_AUTOCOMMIT, connect
 from lp.services.scripts import db_options, logger, logger_options
 
@@ -69,12 +68,11 @@ def main():
         time.sleep(0.6)
         cur.execute(
             """
-            SELECT %(pid)s FROM pg_stat_activity
+            SELECT pid FROM pg_stat_activity
             WHERE
                 datname=current_database()
-                AND %(query)s LIKE 'autovacuum: %%'
+                AND query LIKE 'autovacuum: %'
             """
-            % activity_cols(cur)
         )
         autovacuums = [row[0] for row in cur.fetchall()]
         num_autovacuums = len(autovacuums)
diff --git a/lib/lp/services/database/__init__.py b/lib/lp/services/database/__init__.py
index 015a6f4..a6d4eb1 100644
--- a/lib/lp/services/database/__init__.py
+++ b/lib/lp/services/database/__init__.py
@@ -4,7 +4,6 @@
 """The lp.services.database package."""
 
 __all__ = [
-    "activity_cols",
     "read_transaction",
     "write_transaction",
 ]
@@ -19,11 +18,6 @@ from lp.services.database.sqlbase import reset_store
 RETRY_ATTEMPTS = 3
 
 
-def activity_cols(cur):
-    """Adapt pg_stat_activity column names for the current DB server."""
-    return {"query": "query", "pid": "pid"}
-
-
 def retry_transaction(func):
     """Decorator used to retry database transaction failures.
 
diff --git a/lib/lp/services/looptuner.py b/lib/lp/services/looptuner.py
index 766a765..c6cecb0 100644
--- a/lib/lp/services/looptuner.py
+++ b/lib/lp/services/looptuner.py
@@ -18,7 +18,6 @@ from six import reraise
 from zope.interface import Interface, implementer
 
 import lp.services.scripts
-from lp.services.database import activity_cols
 from lp.services.database.interfaces import IPrimaryStore
 
 
@@ -333,21 +332,18 @@ class DBLoopTuner(LoopTuner):
         while not self._isTimedOut():
             results = list(
                 store.execute(
-                    (
-                        """
+                    """
                 SELECT
                     CURRENT_TIMESTAMP - xact_start,
-                    %(pid)s,
+                    pid,
                     usename,
                     datname,
-                    %(query)s
+                    query
                 FROM activity()
-                WHERE xact_start < CURRENT_TIMESTAMP - interval '%%f seconds'
+                WHERE xact_start < CURRENT_TIMESTAMP - interval '%f seconds'
                     AND datname = current_database()
                 ORDER BY xact_start LIMIT 4
                 """
-                        % activity_cols(store)
-                    )
                     % self.long_running_transaction
                 ).get_all()
             )
diff --git a/lib/lp/testing/pgsql.py b/lib/lp/testing/pgsql.py
index 68aa01f..4f9b92e 100644
--- a/lib/lp/testing/pgsql.py
+++ b/lib/lp/testing/pgsql.py
@@ -18,7 +18,6 @@ from breezy.lock import WriteLock
 from psycopg2.errors import InvalidCatalogName, ObjectInUse
 
 from lp.services.config import config
-from lp.services.database import activity_cols
 
 
 class ConnectionWrapper:
@@ -439,11 +438,10 @@ rw_main_standby: dbname=%s
                     cur = con.cursor()
                     cur.execute(
                         """
-                        SELECT pg_terminate_backend(%(pid)s)
+                        SELECT pg_terminate_backend(pid)
                         FROM pg_stat_activity
-                        WHERE %(pid)s <> pg_backend_pid() AND datname=%%s
-                        """
-                        % activity_cols(cur),
+                        WHERE pid <> pg_backend_pid() AND datname=%s
+                        """,
                         [self.dbname],
                     )
                 except psycopg2.DatabaseError:
diff --git a/test_on_merge.py b/test_on_merge.py
index 99f343d..05fe1cb 100755
--- a/test_on_merge.py
+++ b/test_on_merge.py
@@ -16,8 +16,6 @@ from subprocess import PIPE, STDOUT, Popen
 
 import psycopg2
 
-from lp.services.database import activity_cols
-
 # The TIMEOUT setting (expressed in seconds) affects how long a test will run
 # before it is deemed to be hung, and then appropriately terminated.
 # It's principal use is preventing a PQM job from hanging indefinitely and
@@ -71,12 +69,11 @@ def setup_test_database():
     for loop in range(2):
         cur.execute(
             """
-            SELECT usename, %(query)s
+            SELECT usename, query
             FROM pg_stat_activity
             WHERE datname IN (
                 'launchpad_dev', 'launchpad_ftest_template', 'launchpad_ftest')
             """
-            % activity_cols(cur)
         )
         results = list(cur.fetchall())
         if not results:
diff --git a/utilities/pgkillactive.py b/utilities/pgkillactive.py
index d06c481..c8e5e44 100755
--- a/utilities/pgkillactive.py
+++ b/utilities/pgkillactive.py
@@ -17,8 +17,6 @@ from optparse import OptionParser
 
 import psycopg2
 
-from lp.services.database import activity_cols
-
 
 def main():
     parser = OptionParser()
@@ -75,15 +73,12 @@ def main():
     con = psycopg2.connect(options.connect_string)
     cur = con.cursor()
     cur.execute(
-        (
-            """
-        SELECT usename, %(pid)s, backend_start, xact_start
+        """
+        SELECT usename, pid, backend_start, xact_start
         FROM pg_stat_activity
-        WHERE xact_start < CURRENT_TIMESTAMP - '%%d seconds'::interval %%s
-        ORDER BY %(pid)s
+        WHERE xact_start < CURRENT_TIMESTAMP - '%d seconds'::interval %s
+        ORDER BY pid
         """
-            % activity_cols(cur)
-        )
         % (options.max_seconds, user_match_sql),
         options.users,
     )
diff --git a/utilities/pgkillidle.py b/utilities/pgkillidle.py
index 6dab70b..9985ae8 100755
--- a/utilities/pgkillidle.py
+++ b/utilities/pgkillidle.py
@@ -17,8 +17,6 @@ from optparse import OptionParser
 
 import psycopg2
 
-from lp.services.database import activity_cols
-
 
 def main():
     parser = OptionParser()
@@ -71,16 +69,13 @@ def main():
     con = psycopg2.connect(options.connect_string)
     cur = con.cursor()
     cur.execute(
-        (
-            """
-        SELECT usename, %(pid)s, backend_start, query_start
+        """
+        SELECT usename, pid, backend_start, query_start
         FROM pg_stat_activity
-        WHERE %(query)s = '<IDLE> in transaction'
-            AND query_start < CURRENT_TIMESTAMP - '%%d seconds'::interval %%s
-        ORDER BY %(pid)s
+        WHERE query = '<IDLE> in transaction'
+            AND query_start < CURRENT_TIMESTAMP - '%d seconds'::interval %s
+        ORDER BY pid
         """
-            % activity_cols(cur)
-        )
         % (options.max_idle_seconds, ignore_sql),
         options.ignore,
     )
diff --git a/utilities/pgmassacre.py b/utilities/pgmassacre.py
index deb5793..0939268 100755
--- a/utilities/pgmassacre.py
+++ b/utilities/pgmassacre.py
@@ -24,7 +24,6 @@ import psycopg2
 from psycopg2.extensions import make_dsn, parse_dsn
 
 from lp.services.config import config, dbconfig
-from lp.services.database import activity_cols
 
 
 def connect(dbname="template1"):
@@ -78,11 +77,10 @@ def still_open(database, max_wait=120):
             """
             SELECT TRUE FROM pg_stat_activity
             WHERE
-                datname=%%s
-                AND %(pid)s != pg_backend_pid()
+                datname=%s
+                AND pid != pg_backend_pid()
             LIMIT 1
-            """
-            % activity_cols(cur),
+            """,
             [database],
         )
         if cur.fetchone() is None:
@@ -122,11 +120,10 @@ def massacre(database):
         # Terminate open connections.
         cur.execute(
             """
-            SELECT %(pid)s, pg_terminate_backend(%(pid)s)
+            SELECT pid, pg_terminate_backend(pid)
             FROM pg_stat_activity
-            WHERE datname=%%s AND %(pid)s <> pg_backend_pid()
-            """
-            % activity_cols(cur),
+            WHERE datname=%s AND pid <> pg_backend_pid()
+            """,
             [database],
         )
         for pid, success in cur.fetchall():
@@ -212,11 +209,10 @@ def report_open_connections(database):
         """
         SELECT usename, datname, count(*)
         FROM pg_stat_activity
-        WHERE %(pid)s != pg_backend_pid()
+        WHERE pid != pg_backend_pid()
         GROUP BY usename, datname
         ORDER BY datname, usename
         """
-        % activity_cols(cur)
     )
     for usename, datname, num_connections in cur.fetchall():
         print(