Fix for count being very slow on large Parse Classes' collections (Postgres) (#5330)

* Changed count to be approximate. Should help with postgres slowness * refactored last commit to only fall back to estimate if no complex query * handlign variables correctly * Trying again because it was casting to lowercase table names which doesnt work for us/ * syntax error * Adding quotations to pg query * hopefully final pg fix * Postgres will now use an approximate count unless there is a more complex query specified * handling edge case * Fix for count being very slow on large Parse Classes' collections in Postgres. Replicating fix for Mongo in issue 5264 * Fixed silly spelling error resulting from copying over notes * Lint fixes * limiting results to 1 on approximation * suppress test that we can no longer run for postgres * removed tests from Postgres that no longer apply * made changes requested by dplewis * fixed count errors * updated package.json * removed test exclude for pg * removed object types from method * test disabled for postgres * returned type * add estimate count test * fix mongo test
2019-04-08 15:59:15 -07:00
parent e396612254
commit c7eb7daeae
7 changed files with 181 additions and 57 deletions
--- a/package-lock.json
+++ b/package-lock.json
@@ -2402,14 +2402,14 @@
      "integrity": "sha1-tf1UIgqivFq1eqtxQMlAdUUDwac="
    },
    "cosmiconfig": {
-      "version": "5.1.0",
-      "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-5.1.0.tgz",
-      "integrity": "sha512-kCNPvthka8gvLtzAxQXvWo4FxqRB+ftRZyPZNuab5ngvM9Y7yw7hbEysglptLgpkGX9nAOKTBVkHUAe8xtYR6Q==",
+      "version": "5.2.0",
+      "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-5.2.0.tgz",
+      "integrity": "sha512-nxt+Nfc3JAqf4WIWd0jXLjTJZmsPLrA9DDc4nRw2KFJQJK7DNooqSXrNI7tzLG50CF8axczly5UV929tBmh/7g==",
      "dev": true,
      "requires": {
        "import-fresh": "^2.0.0",
        "is-directory": "^0.3.1",
-        "js-yaml": "^3.9.0",
+        "js-yaml": "^3.13.0",
        "lodash.get": "^4.4.2",
        "parse-json": "^4.0.0"
      }
@@ -2947,14 +2947,14 @@
      }
    },
    "es5-ext": {
-      "version": "0.10.48",
-      "resolved": "https://registry.npmjs.org/es5-ext/-/es5-ext-0.10.48.tgz",
-      "integrity": "sha512-CdRvPlX/24Mj5L4NVxTs4804sxiS2CjVprgCmrgoDkdmjdY4D+ySHa7K3jJf8R40dFg0tIm3z/dk326LrnuSGw==",
+      "version": "0.10.49",
+      "resolved": "https://registry.npmjs.org/es5-ext/-/es5-ext-0.10.49.tgz",
+      "integrity": "sha512-3NMEhi57E31qdzmYp2jwRArIUsj1HI/RxbQ4bgnSB+AIKIxsAmTiK83bYMifIcpWvEc3P1X30DhUKOqEtF/kvg==",
      "dev": true,
      "requires": {
        "es6-iterator": "~2.0.3",
        "es6-symbol": "~3.1.1",
-        "next-tick": "1"
+        "next-tick": "^1.0.0"
      }
    },
    "es6-iterator": {
@@ -3759,6 +3759,12 @@
      "integrity": "sha1-UhTXU3pNBqSjAcDMJi/rhBiAAuc=",
      "dev": true
    },
+    "fn-name": {
+      "version": "2.0.1",
+      "resolved": "https://registry.npmjs.org/fn-name/-/fn-name-2.0.1.tgz",
+      "integrity": "sha1-UhTXU3pNBqSjAcDMJi/rhBiAAuc=",
+      "dev": true
+    },
    "follow-redirects": {
      "version": "1.7.0",
      "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.7.0.tgz",
@@ -4678,6 +4684,32 @@
      "requires": {
        "ajv": "^6.5.5",
        "har-schema": "^2.0.0"
+      },
+      "dependencies": {
+        "ajv": {
+          "version": "6.10.0",
+          "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.10.0.tgz",
+          "integrity": "sha512-nffhOpkymDECQyR0mnsUtoCE8RlX38G0rYP+wgLWFyZuUyuuojSSvi/+euOiQBIn63whYwYVIIH1TvE3tu4OEg==",
+          "dev": true,
+          "requires": {
+            "fast-deep-equal": "^2.0.1",
+            "fast-json-stable-stringify": "^2.0.0",
+            "json-schema-traverse": "^0.4.1",
+            "uri-js": "^4.2.2"
+          }
+        },
+        "fast-deep-equal": {
+          "version": "2.0.1",
+          "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz",
+          "integrity": "sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk=",
+          "dev": true
+        },
+        "json-schema-traverse": {
+          "version": "0.4.1",
+          "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz",
+          "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==",
+          "dev": true
+        }
      }
    },
    "has-ansi": {
@@ -5491,9 +5523,9 @@
      "dev": true
    },
    "js-yaml": {
-      "version": "3.12.2",
-      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.12.2.tgz",
-      "integrity": "sha512-QHn/Lh/7HhZ/Twc7vJYQTkjuCa0kaCcDcjK5Zlk2rvnUpy7DxMJ23+Jc2dcyvltwQVg1nygAVlB2oRDFHoRS5Q==",
+      "version": "3.13.0",
+      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.13.0.tgz",
+      "integrity": "sha512-pZZoSxcCYco+DIKBTimr67J6Hy+EYGZDY/HCWC+iAEA9h1ByhMXAIVUXMcMFpOCxQ/xjXmPI2MkDL5HRm5eFrQ==",
      "dev": true,
      "requires": {
        "argparse": "^1.0.7",
@@ -5692,6 +5724,15 @@
        "graceful-fs": "^4.1.9"
      }
    },
+    "lcid": {
+      "version": "1.0.0",
+      "resolved": "https://registry.npmjs.org/lcid/-/lcid-1.0.0.tgz",
+      "integrity": "sha1-MIrMr6C8SDo4Z7S28rlQYlHRuDU=",
+      "dev": true,
+      "requires": {
+        "invert-kv": "^1.0.0"
+      }
+    },
    "levn": {
      "version": "0.3.0",
      "resolved": "https://registry.npmjs.org/levn/-/levn-0.3.0.tgz",
@@ -9204,9 +9245,9 @@
      "integrity": "sha1-tf3AjxKH6hF4Yo5BXiUTK3NkbG0="
    },
    "simple-git": {
-      "version": "1.107.0",
-      "resolved": "https://registry.npmjs.org/simple-git/-/simple-git-1.107.0.tgz",
-      "integrity": "sha512-t4OK1JRlp4ayKRfcW6owrWcRVLyHRUlhGd0uN6ZZTqfDq8a5XpcUdOKiGRNobHEuMtNqzp0vcJNvhYWwh5PsQA==",
+      "version": "1.110.0",
+      "resolved": "https://registry.npmjs.org/simple-git/-/simple-git-1.110.0.tgz",
+      "integrity": "sha512-UYY0rQkknk0P5eb+KW+03F4TevZ9ou0H+LoGaj7iiVgpnZH4wdj/HTViy/1tNNkmIPcmtxuBqXWiYt2YwlRKOQ==",
      "dev": true,
      "requires": {
        "debug": "^4.0.1"
@@ -9651,6 +9692,12 @@
      "integrity": "sha512-TyOuWLwkmtPL49LHCX1caIwHjRzcVd62+GF6h8W/jHOeZUFHpnd2XJDVuUlaTaLPH1nuu2M69mfHr5XbQJnf/g==",
      "dev": true
    },
+    "synchronous-promise": {
+      "version": "2.0.6",
+      "resolved": "https://registry.npmjs.org/synchronous-promise/-/synchronous-promise-2.0.6.tgz",
+      "integrity": "sha512-TyOuWLwkmtPL49LHCX1caIwHjRzcVd62+GF6h8W/jHOeZUFHpnd2XJDVuUlaTaLPH1nuu2M69mfHr5XbQJnf/g==",
+      "dev": true
+    },
    "table": {
      "version": "5.2.3",
      "resolved": "https://registry.npmjs.org/table/-/table-5.2.3.tgz",
--- a/spec/AudienceRouter.spec.js
+++ b/spec/AudienceRouter.spec.js
@@ -145,7 +145,7 @@ describe('AudiencesRouter', () => {
      });
  });

-  it('query installations with count = 1', done => {
+  it_exclude_dbs(['postgres'])('query installations with count = 1', done => {
    const config = Config.get('test');
    const androidAudienceRequest = {
      name: 'Android Users',
@@ -189,7 +189,7 @@ describe('AudiencesRouter', () => {
      });
  });

-  it('query installations with limit = 0 and count = 1', done => {
+  it_exclude_dbs(['postgres'])('query installations with limit = 0 and count = 1', done => {
    const config = Config.get('test');
    const androidAudienceRequest = {
      name: 'Android Users',
--- a/spec/InstallationsRouter.spec.js
+++ b/spec/InstallationsRouter.spec.js
@@ -160,7 +160,7 @@ describe('InstallationsRouter', () => {
      });
  });

-  it('query installations with count = 1', done => {
+  it_exclude_dbs(['postgres'])('query installations with count = 1', done => {
    const config = Config.get('test');
    const androidDeviceRequest = {
      installationId: '12345678-abcd-abcd-abcd-123456789abc',
@@ -209,7 +209,7 @@ describe('InstallationsRouter', () => {
      });
  });

-  it('query installations with limit = 0 and count = 1', done => {
+  it_only_db('postgres')('query installations with count = 1', async () => {
    const config = Config.get('test');
    const androidDeviceRequest = {
      installationId: '12345678-abcd-abcd-abcd-123456789abc',
@@ -224,40 +224,90 @@ describe('InstallationsRouter', () => {
      auth: auth.master(config),
      body: {},
      query: {
-        limit: 0,
        count: 1,
      },
      info: {},
    };

    const router = new InstallationsRouter();
-    rest
-      .create(
-        config,
-        auth.nobody(config),
-        '_Installation',
-        androidDeviceRequest
-      )
-      .then(() => {
-        return rest.create(
+    await rest.create(
+      config,
+      auth.nobody(config),
+      '_Installation',
+      androidDeviceRequest
+    );
+    await rest.create(
+      config,
+      auth.nobody(config),
+      '_Installation',
+      iosDeviceRequest
+    );
+    let res = await router.handleFind(request);
+    let response = res.response;
+    expect(response.results.length).toEqual(2);
+    expect(response.count).toEqual(0); // estimate count is zero
+
+    const pgAdapter = config.database.adapter;
+    await pgAdapter.updateEstimatedCount('_Installation');
+
+    res = await router.handleFind(request);
+    response = res.response;
+    expect(response.results.length).toEqual(2);
+    expect(response.count).toEqual(2);
+  });
+
+  it_exclude_dbs(['postgres'])(
+    'query installations with limit = 0 and count = 1',
+    done => {
+      const config = Config.get('test');
+      const androidDeviceRequest = {
+        installationId: '12345678-abcd-abcd-abcd-123456789abc',
+        deviceType: 'android',
+      };
+      const iosDeviceRequest = {
+        installationId: '12345678-abcd-abcd-abcd-123456789abd',
+        deviceType: 'ios',
+      };
+      const request = {
+        config: config,
+        auth: auth.master(config),
+        body: {},
+        query: {
+          limit: 0,
+          count: 1,
+        },
+        info: {},
+      };
+
+      const router = new InstallationsRouter();
+      rest
+        .create(
          config,
          auth.nobody(config),
          '_Installation',
-          iosDeviceRequest
-        );
-      })
-      .then(() => {
-        return router.handleFind(request);
-      })
-      .then(res => {
-        const response = res.response;
-        expect(response.results.length).toEqual(0);
-        expect(response.count).toEqual(2);
-        done();
-      })
-      .catch(err => {
-        fail(JSON.stringify(err));
-        done();
-      });
-  });
+          androidDeviceRequest
+        )
+        .then(() => {
+          return rest.create(
+            config,
+            auth.nobody(config),
+            '_Installation',
+            iosDeviceRequest
+          );
+        })
+        .then(() => {
+          return router.handleFind(request);
+        })
+        .then(res => {
+          const response = res.response;
+          expect(response.results.length).toEqual(0);
+          expect(response.count).toEqual(2);
+          done();
+        })
+        .catch(err => {
+          fail(JSON.stringify(err));
+          done();
+        });
+    }
+  );
 });
--- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js
+++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js
@@ -152,10 +152,7 @@ export class MongoStorageAdapter implements StorageAdapter {
    // encoded
    const encodedUri = formatUrl(parseUrl(this._uri));

-    this.connectionPromise = MongoClient.connect(
-      encodedUri,
-      this._mongoOptions
-    )
+    this.connectionPromise = MongoClient.connect(encodedUri, this._mongoOptions)
      .then(client => {
        // Starting mongoDB 3.0, the MongoClient.connect don't return a DB anymore but a client
        // Fortunately, we can get back the options and use them to select the proper DB.
@@ -385,8 +382,8 @@ export class MongoStorageAdapter implements StorageAdapter {
  deleteAllClasses(fast: boolean) {
    return storageAdapterAllCollections(this).then(collections =>
      Promise.all(
-        collections.map(
-          collection => (fast ? collection.deleteMany({}) : collection.drop())
+        collections.map(collection =>
+          fast ? collection.deleteMany({}) : collection.drop()
        )
      )
    );
@@ -952,6 +949,8 @@ export class MongoStorageAdapter implements StorageAdapter {
        readPreference = ReadPreference.NEAREST;
        break;
      case undefined:
+      case null:
+      case '':
        break;
      default:
        throw new Parse.Error(
--- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
+++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
@@ -1962,17 +1962,37 @@ export class PostgresStorageAdapter implements StorageAdapter {
  }

  // Executes a count.
-  count(className: string, schema: SchemaType, query: QueryType) {
-    debug('count', className, query);
+  count(
+    className: string,
+    schema: SchemaType,
+    query: QueryType,
+    readPreference?: string,
+    estimate?: boolean = true
+  ) {
+    debug('count', className, query, readPreference, estimate);
    const values = [className];
    const where = buildWhereClause({ schema, query, index: 2 });
    values.push(...where.values);

    const wherePattern =
      where.pattern.length > 0 ? `WHERE ${where.pattern}` : '';
-    const qs = `SELECT count(*) FROM $1:name ${wherePattern}`;
+    let qs = '';
+
+    if (where.pattern.length > 0 || !estimate) {
+      qs = `SELECT count(*) FROM $1:name ${wherePattern}`;
+    } else {
+      qs =
+        'SELECT reltuples AS approximate_row_count FROM pg_class WHERE relname = $1';
+    }
+
    return this._client
-      .one(qs, values, a => +a.count)
+      .one(qs, values, a => {
+        if (a.approximate_row_count != null) {
+          return +a.approximate_row_count;
+        } else {
+          return +a.count;
+        }
+      })
      .catch(error => {
        if (error.code !== PostgresRelationDoesNotExistError) {
          throw error;
@@ -2327,6 +2347,11 @@ export class PostgresStorageAdapter implements StorageAdapter {
  updateSchemaWithIndexes(): Promise<void> {
    return Promise.resolve();
  }
+
+  // Used for testing purposes
+  updateEstimatedCount(className: string) {
+    return this._client.none('ANALYZE $1:name', [className]);
+  }
 }

 function convertPolygonToSQL(polygon) {
--- a/src/Adapters/Storage/StorageAdapter.js
+++ b/src/Adapters/Storage/StorageAdapter.js
@@ -86,7 +86,8 @@ export interface StorageAdapter {
    className: string,
    schema: SchemaType,
    query: QueryType,
-    readPreference: ?string
+    readPreference?: string,
+    estimate?: boolean
  ): Promise<number>;
  distinct(
    className: string,
--- a/src/Controllers/DatabaseController.js
+++ b/src/Controllers/DatabaseController.js
@@ -1324,7 +1324,9 @@ class DatabaseController {
      })
      .then((schema: any) => {
        return this.collectionExists(className)
-          .then(() => this.adapter.count(className, { fields: {} }))
+          .then(() =>
+            this.adapter.count(className, { fields: {} }, null, '', false)
+          )
          .then(count => {
            if (count > 0) {
              throw new Parse.Error(