fix abysmal postgresql performance in getSortedSetsMembers (#8030)

* refactor postgres upgrade function to use async * add function wrapper for getting the elements of a sorted set to avoid postgres planning the query with a nested sequential scan * fix fatal(?!) lint errors * add missing await * bump PostgreSQL version on Travis CI to one that isn't over 3 years out of date
5 years ago · f65922297d
parent b47f1769e4
commit f65922297d
3 changed files with 76 additions and 67 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -28,7 +28,7 @@ addons:
    packages:
      - g++-4.8
      - mongodb-org-server
-  postgresql: "9.5"
+  postgresql: "12"
 node_js:
  - "12"
  - "10"
--- a/src/database/postgres.js
+++ b/src/database/postgres.js
@ -63,7 +63,10 @@ postgresModule.init = function (callback) {
 		postgresModule.pool = db;
 		postgresModule.client = db;

-		checkUpgrade(client, function (err) {
+		checkUpgrade(client).then(function () {
+			release();
+			callback(null);
+		}, function (err) {
 			release();
 			callback(err);
 		});
@ -71,8 +74,8 @@ postgresModule.init = function (callback) {
 };


-function checkUpgrade(client, callback) {
-	client.query(`
+async function checkUpgrade(client) {
+	var res = await client.query(`
 SELECT EXISTS(SELECT *
                FROM "information_schema"."columns"
               WHERE "table_schema" = 'public'
@ -82,32 +85,32 @@ SELECT EXISTS(SELECT *
                FROM "information_schema"."columns"
               WHERE "table_schema" = 'public'
                 AND "table_name" = 'legacy_hash'
-                 AND "column_name" = '_key') b`, function (err, res) {
-		if (err) {
-			return callback(err);
-		}
-
-		if (res.rows[0].b) {
-			return callback(null);
-		}
+                 AND "column_name" = '_key') b,
+       EXISTS(SELECT *
+                FROM "information_schema"."routines"
+               WHERE "routine_schema" = 'public'
+                 AND "routine_name" = 'nodebb_get_sorted_set_members') c`);

-		var query = client.query.bind(client);
+	if (res.rows[0].a && res.rows[0].b && res.rows[0].c) {
+		return;
+	}

-		async.series([
-			async.apply(query, `BEGIN`),
-			async.apply(query, `
+	await client.query(`BEGIN`);
+	try {
+		if (!res.rows[0].b) {
+			await client.query(`
 CREATE TYPE LEGACY_OBJECT_TYPE AS ENUM (
 	'hash', 'zset', 'set', 'list', 'string'
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_object" (
 	"_key" TEXT NOT NULL
 		PRIMARY KEY,
 	"type" LEGACY_OBJECT_TYPE NOT NULL,
 	"expireAt" TIMESTAMPTZ DEFAULT NULL,
 	UNIQUE ( "_key", "type" )
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_hash" (
 	"_key" TEXT NOT NULL
 		PRIMARY KEY,
@ -120,8 +123,8 @@ CREATE TABLE "legacy_hash" (
 		REFERENCES "legacy_object"("_key", "type")
 		ON UPDATE CASCADE
 		ON DELETE CASCADE
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_zset" (
 	"_key" TEXT NOT NULL,
 	"value" TEXT NOT NULL,
@ -135,8 +138,8 @@ CREATE TABLE "legacy_zset" (
 		REFERENCES "legacy_object"("_key", "type")
 		ON UPDATE CASCADE
 		ON DELETE CASCADE
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_set" (
 	"_key" TEXT NOT NULL,
 	"member" TEXT NOT NULL,
@ -149,8 +152,8 @@ CREATE TABLE "legacy_set" (
 		REFERENCES "legacy_object"("_key", "type")
 		ON UPDATE CASCADE
 		ON DELETE CASCADE
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_list" (
 	"_key" TEXT NOT NULL
 		PRIMARY KEY,
@ -163,8 +166,8 @@ CREATE TABLE "legacy_list" (
 		REFERENCES "legacy_object"("_key", "type")
 		ON UPDATE CASCADE
 		ON DELETE CASCADE
-)`),
-			async.apply(query, `
+)`);
+			await client.query(`
 CREATE TABLE "legacy_string" (
 	"_key" TEXT NOT NULL
 		PRIMARY KEY,
@ -177,13 +180,10 @@ CREATE TABLE "legacy_string" (
 		REFERENCES "legacy_object"("_key", "type")
 		ON UPDATE CASCADE
 		ON DELETE CASCADE
-)`),
-			function (next) {
-				if (!res.rows[0].a) {
-					return next();
-				}
-				async.series([
-					async.apply(query, `
+)`);
+
+			if (res.rows[0].a) {
+				await client.query(`
 INSERT INTO "legacy_object" ("_key", "type", "expireAt")
 SELECT DISTINCT "data"->>'_key',
                CASE WHEN (SELECT COUNT(*)
@ -210,8 +210,8 @@ SELECT DISTINCT "data"->>'_key',
                     THEN to_timestamp(("data"->>'expireAt')::double precision / 1000)
                     ELSE NULL
                END
-  FROM "objects"`),
-					async.apply(query, `
+  FROM "objects"`);
+				await client.query(`
 INSERT INTO "legacy_hash" ("_key", "data")
 SELECT "data"->>'_key',
       "data" - '_key' - 'expireAt'
@ -227,8 +227,8 @@ SELECT "data"->>'_key',
            THEN NOT (("data" ? 'value')
                  AND ("data" ? 'score'))
            ELSE TRUE
-       END`),
-					async.apply(query, `
+       END`);
+				await client.query(`
 INSERT INTO "legacy_zset" ("_key", "value", "score")
 SELECT "data"->>'_key',
       "data"->>'value',
@ -237,16 +237,16 @@ SELECT "data"->>'_key',
 WHERE (SELECT COUNT(*)
          FROM jsonb_object_keys("data" - 'expireAt')) = 3
   AND ("data" ? 'value')
-   AND ("data" ? 'score')`),
-					async.apply(query, `
+   AND ("data" ? 'score')`);
+				await client.query(`
 INSERT INTO "legacy_set" ("_key", "member")
 SELECT "data"->>'_key',
       jsonb_array_elements_text("data"->'members')
  FROM "objects"
 WHERE (SELECT COUNT(*)
          FROM jsonb_object_keys("data" - 'expireAt')) = 2
-   AND ("data" ? 'members')`),
-					async.apply(query, `
+   AND ("data" ? 'members')`);
+				await client.query(`
 INSERT INTO "legacy_list" ("_key", "array")
 SELECT "data"->>'_key',
       ARRAY(SELECT t
@ -255,8 +255,8 @@ SELECT "data"->>'_key',
  FROM "objects"
 WHERE (SELECT COUNT(*)
          FROM jsonb_object_keys("data" - 'expireAt')) = 2
-   AND ("data" ? 'array')`),
-					async.apply(query, `
+   AND ("data" ? 'array')`);
+				await client.query(`
 INSERT INTO "legacy_string" ("_key", "data")
 SELECT "data"->>'_key',
       CASE WHEN "data" ? 'value'
@ -267,23 +267,37 @@ SELECT "data"->>'_key',
 WHERE (SELECT COUNT(*)
          FROM jsonb_object_keys("data" - 'expireAt')) = 2
   AND (("data" ? 'value')
-     OR ("data" ? 'data'))`),
-					async.apply(query, `DROP TABLE "objects" CASCADE`),
-					async.apply(query, `DROP FUNCTION "fun__objects__expireAt"() CASCADE`),
-				], next);
-			},
-			async.apply(query, `
+     OR ("data" ? 'data'))`);
+				await client.query(`DROP TABLE "objects" CASCADE`);
+				await client.query(`DROP FUNCTION "fun__objects__expireAt"() CASCADE`);
+			}
+			await client.query(`
 CREATE VIEW "legacy_object_live" AS
 SELECT "_key", "type"
  FROM "legacy_object"
 WHERE "expireAt" IS NULL
-    OR "expireAt" > CURRENT_TIMESTAMP`),
-		], function (err) {
-			query(err ? `ROLLBACK` : `COMMIT`, function (err1) {
-				callback(err1 || err);
-			});
-		});
-	});
+    OR "expireAt" > CURRENT_TIMESTAMP`);
+		}
+
+		if (!res.rows[0].c) {
+			await client.query(`
+CREATE FUNCTION "nodebb_get_sorted_set_members"(TEXT) RETURNS TEXT[] AS $$
+    SELECT array_agg(z."value" ORDER BY z."score" ASC)
+      FROM "legacy_object_live" o
+     INNER JOIN "legacy_zset" z
+             ON o."_key" = z."_key"
+            AND o."type" = z."type"
+          WHERE o."_key" = $1
+$$ LANGUAGE sql
+STABLE
+STRICT
+PARALLEL SAFE`);
+		}
+	} catch (ex) {
+		await client.query(`ROLLBACK`);
+		throw ex;
+	}
+	await client.query(`COMMIT`);
 }

 postgresModule.createSessionStore = function (options, callback) {
--- a/src/database/postgres/sorted.js
+++ b/src/database/postgres/sorted.js
@ -463,19 +463,14 @@ SELECT o."_key" k
 		const res = await module.pool.query({
 			name: 'getSortedSetsMembers',
 			text: `
-SELECT o."_key" k,
-       array_agg(z."value" ORDER BY z."score" ASC) m
-  FROM "legacy_object_live" o
- INNER JOIN "legacy_zset" z
-         ON o."_key" = z."_key"
-        AND o."type" = z."type"
- WHERE o."_key" = ANY($1::TEXT[])
- GROUP BY o."_key"`,
+SELECT "_key" k,
+       "nodebb_get_sorted_set_members"("_key") m
+  FROM UNNEST($1::TEXT[]) "_key";`,
 			values: [keys],
 		});

 		return keys.map(function (k) {
-			return (res.rows.find(r => r.k === k) || { m: [] }).m;
+			return (res.rows.find(r => r.k === k) || {}).m || [];
 		});
 	};