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

CoderickLamar · 2019-01-29T22:37:14Z

Fetching the count for Postgres tables that are large, >5 mil in our case, takes an extremely long time that bogs down the server. This is a known issue with Postgres: https://wiki.postgresql.org/wiki/Slow_Counting

If no complex query is supplied, this will now default to estimating the count which is accurate enough for parse dashboard.

This is an equivalent fix to commit 5264 which solved this for MongoDb.

…esnt work for us/

…plex query specified

…Postgres. Replicating fix for Mongo in issue 5264

codecov · 2019-01-29T23:02:53Z

Codecov Report

Merging #5330 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5330      +/-   ##
==========================================
- Coverage   93.96%   93.95%   -0.01%     
==========================================
  Files         123      124       +1     
  Lines        9027     9056      +29     
==========================================
+ Hits         8482     8509      +27     
- Misses        545      547       +2

Impacted Files	Coverage Δ
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	`91.48% <100%> (-0.73%)`	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`97.09% <100%> (+0.01%)`	⬆️
src/Controllers/DatabaseController.js	`95.08% <100%> (ø)`	⬆️
src/RestWrite.js	`93.07% <0%> (-0.37%)`	⬇️
src/Adapters/Cache/RedisCacheAdapter.js
src/Adapters/Cache/RedisCacheAdapter/index.js	`100% <0%> (ø)`
...dapters/Cache/RedisCacheAdapter/KeyPromiseQueue.js	`95.45% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3c8629...8951580. Read the comment docs.

flovilmart · 2019-01-29T23:57:36Z

That’s a great idea. Seems that a few PG tests broke along the way. Can you have a look?

_{Sent with GitHawk}

CoderickLamar · 2019-01-30T00:27:31Z

Thoughts on the broken tests?

7740.3:

    ✗ query installations with count = 1 (0.05 sec)
      - Expected 0 to equal 2.
    ✗ query installations with limit = 0 and count = 1 (0.045 sec)
      - Expected 0 to equal 2.

I believe the reason this fails is because the postgres database is freshly created and hasn't had a chance to have the pg_class table built... since this is an estimate and not an exact value.

Unsure of how 7740.5 relates to count.

flovilmart · 2019-01-30T00:42:45Z

Thoughts on the broken tests? ...

@CoderickLamar we can disable this test on PG specifically if we need to (look for it_only_db) or we could let pg get enough time

_{Sent with GitHawk}

ben-sab · 2019-01-30T23:28:22Z

@flovilmart, we've disabled the tests for PG as suggested but we're getting new errors which don't seem to be related to the previous ones. I was wondering if you could point us in the right direction by any chance? Thanks.

✗ refuses to delete non-empty collection (20 secs)
      - Failed: HTTPResponse({ status: 200, headers: Object({ x-powered-by: 'Express', access-control-allow-origin: '*', access-control-allow-methods: 'GET,PUT,POST,DELETE,OPTIONS', access-control-allow-headers: 'X-Parse-Master-Key, X-Parse-REST-API-Key, X-Parse-Javascript-Key, X-Parse-Application-Id, X-Parse-Client-Version, X-Parse-Session-Token, X-Requested-With, X-Parse-Revocable-Session, Content-Type', access-control-expose-headers: 'X-Parse-Job-Status-Id, X-Parse-Push-Status-Id', content-type: 'application/json; charset=utf-8', content-length: '2', etag: 'W/"2-vyGp6PvFo4RvsFtPoIWeCReyIC8"', date: 'Wed, 30 Jan 2019 22:57:11 GMT', connection: 'close' }), cookies: undefined, buffer: Buffer [ 123, 125 ], text: <getter>, data: <getter> })
      - Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

✗ does not fail when deleting nonexistant collections (20 secs)
      - Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

flovilmart · 2019-01-30T23:48:55Z

Usually, when a test hit the timeout, it’s because done is not called in time. This can be solved by handling all promises rejections with a done.fail. Does this make sense?

_{Sent with GitHawk}

dplewis · 2019-01-30T23:55:36Z

@CoderickLamar I looked this over real quick.

I believe the reason this fails is because the postgres database is freshly created and hasn't had a chance to have the pg_class table built... since this is an estimate and not an exact value.

Based on the wiki you posted, the estimate value updates on certain events.

If you would have ran analyze _Installation or waited for the autovacuum to periodically update the estimate the test would have passed . (Note you must have autovacuum enabled on Postgres for this estimate count to work, it is defaulted to on).

I think as a fall back looking at the failing test.

it('query installations with count = 1', done => {
  ...
  const response = res.response;
  expect(response.results.length).toEqual(2);
  expect(response.count).toEqual(2); // returns 0 could be assign with response.results.length
  ...
});

dplewis

Just a few nits from me

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

ben-sab · 2019-02-01T00:57:09Z

Thanks for your reply. @dplewis I have made the changes requested. As for the errors which still persist, on my local they are passing fine and they're not taking long either:

Do we need to increase the timeout?

flovilmart · 2019-02-01T01:35:36Z

Perhaps a different Postgres version?

_{Sent with GitHawk}

dplewis · 2019-02-01T02:10:48Z

What version are running I tried it on 9.6 and 10 and it works for me locally

flovilmart · 2019-02-01T02:34:36Z

Travis runs 9.5

_{Sent with GitHawk}

dplewis · 2019-02-01T02:40:01Z

@flovilmart I’ll try 9.5 but I don’t know how this would affect the schema. I’ll try tomorrow

ben-sab · 2019-02-01T18:32:26Z

@flovilmart @dplewis I was running PG 11 on my local.

CoderickLamar · 2019-02-02T22:31:12Z

@dplewis @flovilmart How can we get postgres updated on travis?

flovilmart · 2019-02-02T22:43:20Z

We should not, instead the fix should be compatible with Postgres 9.5.

_{Sent with GitHawk}

CoderickLamar · 2019-02-05T00:30:13Z

On that note, we should figure out the best route forward. We should get this test passing, but it sounds like that is separate task?

flovilmart · 2019-02-05T00:44:23Z

The tests that are failing on this branch are passing on the master branch. So the changes introduce a possible regression.

_{Sent with GitHawk}

flovilmart · 2019-02-05T00:45:40Z

Also, excluding the tests is not something I am a big fan of, as we should maintain the same level of features on all databases

_{Sent with GitHawk}

dplewis · 2019-02-05T19:29:52Z

@CoderickLamar As I mentioned above #5330 (comment) estimate count will always be 0 until it is updated by an event.

I was able to get the failing test to work locally somehow.

refuses to delete non-empty collection test fails because this will use estimate count and the collection will always report empty. You can delete an empty collection.

Same goes for does not fail when deleting nonexistant collections

This means things like deleting from schema needs a hard count or somewhat close estimation .

CoderickLamar · 2019-03-16T22:20:25Z

@ben-sab @dplewis
Solution here seems to be to let the estimation rebuild

Another solution would be to implement hard count specifically for deleting schema... can someone point me in the right direction for that?

dplewis · 2019-03-27T05:03:05Z

@behrangs Thank you for the fix. Simple and beautiful.

Can you resolve the conflicts? There is a dependency bot that handles updating packages

CoderickLamar · 2019-04-01T05:37:12Z

What are the next steps here?

spec/InstallationsRouter.spec.js

dplewis · 2019-04-03T02:56:45Z

@CoderickLamar @ben-sab Can you try this branch with your projects? I don't know how long autovacuum takes on your machines for you to see an estimated count thats not zero.

CoderickLamar · 2019-04-05T02:54:48Z

Everything is looking good on my end. Speedy dashboard once again.

CoderickLamar · 2019-04-08T19:49:42Z

Can we get this merged into a release?

CoderickLamar · 2019-04-15T20:35:28Z

How often are releases usually put together on master?

…stgres) (parse-community#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

Sagar Malhi added 11 commits January 27, 2019 19:13

Changed count to be approximate. Should help with postgres slowness

ec332d7

refactored last commit to only fall back to estimate if no complex query

9e2e8cd

handlign variables correctly

2ac2e82

Trying again because it was casting to lowercase table names which do…

7bf3679

…esnt work for us/

syntax error

bd6e816

Adding quotations to pg query

3209b5d

hopefully final pg fix

fb47e0f

Postgres will now use an approximate count unless there is a more com…

1d90ed4

…plex query specified

handling edge case

87f9add

Fix for count being very slow on large Parse Classes' collections in …

909243d

…Postgres. Replicating fix for Mongo in issue 5264

Fixed silly spelling error resulting from copying over notes

6be92f1

Lint fixes

f464f51

Sagar Malhi and others added 3 commits January 30, 2019 10:25

limiting results to 1 on approximation

7b55d87

suppress test that we can no longer run for postgres

662ed81

removed tests from Postgres that no longer apply

b29d93b

dplewis suggested changes Jan 31, 2019

View reviewed changes

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js Outdated Show resolved Hide resolved

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js Outdated Show resolved Hide resolved

made changes requested by dplewis

15991a3

behrangs added 2 commits March 26, 2019 21:37

fixed count errors

7401970

updated package.json

9136b20

Merge branch 'master' into bugfix-slowPostgresCount

fcf63bd

dplewis suggested changes Apr 2, 2019

View reviewed changes

spec/InstallationsRouter.spec.js Show resolved Hide resolved

behrangs and others added 6 commits April 2, 2019 16:17

removed test exclude for pg

25597b0

removed object types from method

9bc37cf

test disabled for postgres

00c23c4

returned type

7d63f80

add estimate count test

f397c8a

fix mongo test

8951580

dplewis requested a review from acinader April 3, 2019 03:11

acinader removed their request for review April 3, 2019 03:42

dplewis requested a review from vitaly-t April 3, 2019 20:48

dplewis merged commit c7eb7da into parse-community:master Apr 8, 2019

dplewis mentioned this pull request Jan 3, 2020

Wrong Object Count in PostgreSQL #6311

Closed

dplewis mentioned this pull request Aug 30, 2022

Object count always zero parse-community/parse-dashboard#2247

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

CoderickLamar commented Jan 29, 2019 •

edited

Loading

codecov bot commented Jan 29, 2019 •

edited

Loading

flovilmart commented Jan 29, 2019

CoderickLamar commented Jan 30, 2019

flovilmart commented Jan 30, 2019

ben-sab commented Jan 30, 2019

flovilmart commented Jan 30, 2019

dplewis commented Jan 30, 2019

dplewis left a comment

ben-sab commented Feb 1, 2019

flovilmart commented Feb 1, 2019

dplewis commented Feb 1, 2019

flovilmart commented Feb 1, 2019

dplewis commented Feb 1, 2019

ben-sab commented Feb 1, 2019

CoderickLamar commented Feb 2, 2019

flovilmart commented Feb 2, 2019

CoderickLamar commented Feb 5, 2019

flovilmart commented Feb 5, 2019 •

edited

Loading

flovilmart commented Feb 5, 2019

dplewis commented Feb 5, 2019 •

edited

Loading

CoderickLamar commented Mar 16, 2019

dplewis commented Mar 27, 2019 •

edited

Loading

CoderickLamar commented Apr 1, 2019

dplewis commented Apr 3, 2019 •

edited

Loading

CoderickLamar commented Apr 5, 2019

CoderickLamar commented Apr 8, 2019

CoderickLamar commented Apr 15, 2019

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

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

Conversation

CoderickLamar commented Jan 29, 2019 • edited Loading

codecov bot commented Jan 29, 2019 • edited Loading

Codecov Report

flovilmart commented Jan 29, 2019

CoderickLamar commented Jan 30, 2019

flovilmart commented Jan 30, 2019

ben-sab commented Jan 30, 2019

flovilmart commented Jan 30, 2019

dplewis commented Jan 30, 2019

dplewis left a comment

Choose a reason for hiding this comment

ben-sab commented Feb 1, 2019

flovilmart commented Feb 1, 2019

dplewis commented Feb 1, 2019

flovilmart commented Feb 1, 2019

dplewis commented Feb 1, 2019

ben-sab commented Feb 1, 2019

CoderickLamar commented Feb 2, 2019

flovilmart commented Feb 2, 2019

CoderickLamar commented Feb 5, 2019

flovilmart commented Feb 5, 2019 • edited Loading

flovilmart commented Feb 5, 2019

dplewis commented Feb 5, 2019 • edited Loading

CoderickLamar commented Mar 16, 2019

dplewis commented Mar 27, 2019 • edited Loading

CoderickLamar commented Apr 1, 2019

dplewis commented Apr 3, 2019 • edited Loading

CoderickLamar commented Apr 5, 2019

CoderickLamar commented Apr 8, 2019

CoderickLamar commented Apr 15, 2019

CoderickLamar commented Jan 29, 2019 •

edited

Loading

codecov bot commented Jan 29, 2019 •

edited

Loading

flovilmart commented Feb 5, 2019 •

edited

Loading

dplewis commented Feb 5, 2019 •

edited

Loading

dplewis commented Mar 27, 2019 •

edited

Loading

dplewis commented Apr 3, 2019 •

edited

Loading