CUMULUS-3642: postgres query adjustment #3731

jennyhliu · 2024-07-16T20:17:21Z

Summary: Summary of changes

Addresses CUMULUS-3642 Granules, collections, execution and stats performance test and query adjustment with large scale database

Changes

Adjusted queries to improve performance:
- Used count(*) over count(id) to count rows
- Estimated row count for large tables (granules and executions) by default for basic query
Updated stats summary to default to the last day
Updated ExecutionSearch to not include asyncOperationId by default
Fixed lint
Put back a few logic in master back to granules endpoints

PR Checklist

Update CHANGELOG
Unit tests
Ad-hoc testing - Deploy changes and test manually
Integration tests

packages/api/endpoints/granules.js

This reverts commit 88024c2.

Nnaga1 · 2024-07-30T16:52:26Z

packages/api/endpoints/stats.js

+    now - 24 * 3600 * 1000
+  ), 10);
+  params.timestamp__to = Number.parseInt(get(params, 'timestamp__to', now), 10);
+  const stats = new StatsSearch({ queryStringParameters: params }, 'granule');


so now, it gets the recent day of granule stats summary, can they still have it where it shows the summary of all time? or would that be too slow/time-consuming api-reponse-wise

The aggregations (count, avg) take forever without limiting the time range

makes sense, it wasn't returning for me when using the lp-db in most cases, thanks for the explanation 👍

Nnaga1 · 2024-07-30T19:47:04Z

packages/db/src/search/BaseSearch.ts

+   * @returns whether an estimated row count should be returned
+   */
+  protected shouldEstimateRowcount(countSql: string): boolean {
+    const isBasicQuery = (countSql === `select count(*) from "${this.tableName}"`);


so itll do an estimate if its a basic query, without any where, from, range, etc. ?

Yeah, it's for returning the table rowcount. With filters, we can't estimate the count and have to run the query.

packages/db/src/search/BaseSearch.ts

packages/db/src/search/CollectionSearch.ts

packages/db/src/search/ExecutionSearch.ts

Nnaga1 · 2024-07-30T19:56:21Z

packages/db/src/search/StatsSearch.ts

@@ -72,8 +77,8 @@ class StatsSearch extends BaseSearch {
  /**
   * Formats the postgres records into an API stats/aggregate response
   *
-   * @param {Record<string, Aggregate>} result - the postgres query results


whyd you remove the jsdoc stuff for StatsSearch? just wondering what the reasoning was

These cause eslint errors in vscode.

Nnaga1 · 2024-07-30T20:27:25Z

packages/db/tests/search/test-ExecutionSearch.js

@@ -145,6 +147,7 @@ test('ExecutionSearch returns correct response for basic query', async (t) => {
 test('ExecutionSearch supports page and limit params', async (t) => {
  const { knex } = t.context;
  let queryStringParameters = {
+    estimateTableRowCount: 'false',


so estimateTableRowCount is by default true unless its specifically passed in as false?

For execution and granules, estimateTableRowCount is set to true by default, unless it's passed in as false. I set them to false in these tests, so that the accurate count is returned.

Nnaga1

Thanks for answering my questions, this looks great! Great job as usual 🚀 🎊

Nnaga1 · 2024-07-31T14:37:09Z

packages/api/tests/endpoints/stats.js

@@ -209,17 +211,17 @@ test('GET /stats/aggregate with an invalid access token returns an unauthorized
  assertions.isInvalidAccessTokenResponse(t, response);
 });

-test('GET /stats returns correct response, defaulted to all', async (t) => {
+test('GET /stats returns correct response, defaulted to the last day', async (t) => {


one small nit, instead of the last day id say the last 24 hours, it makes it more clear its from the recent past versus some time frame, not really breaking though

* CUMULUS-3692: Update granules List endpoints to query postgres for basic queries (#3637) * CUMULUS-3692:Granule list endpoint for basic postgres query * CUMULUS-3694: Update granules List endpoints to query postgres - filter by field value (#3656) * CUMULUS-3692:Granule list endpoint for basic postgres query * refactor * refactor * typing * add changelog entry * skip search_after * skip searchafter unit tests * add granule list test * rename * refactor * build query parameters * update comment * add field-mapping * update jsdoc * use type over interface,add log * update test description * build term/terms * buildDbQueryParameters * add unit test no terms search * add doc * rename * add unit test * add fields test * add more unit tests * support error.Error search * fix lint * rename functions * ignore files * add convert query unit tests * add all types * add unit test for fieldmapping types fix timestamp * update timestamp test * add multiple term field test * ignore execution in granule list record * CUMULUS-3689: Update Stats/Summary and Stats/Aggregate endpoints to use psql (#3659) * first commit on new branch * CHANGELOG change * small fix * PR feedback * adding jsdoc + fixing spelling/grammar * CUMULUS-3693: Update granules List endpoints to query postgres - range (#3660) * add range query support * CUMULUS-3695 - Update Granules endpoint to handle SortFields (#3663) * first committ * CHANGELOG change * fixing sortQueryMethod * simplifying code * PR feedback * merge conflicts + improving code * small jsdoc fix * PR feedback * PR feedback * PR feedback * fixing test * PR feedback * PR feedback * CUMULUS-3696: Update granules List endpoints to query postgres - match (#3674) * add methods to convert terms,not,exists * CUMULUS-3641 - Update Collections LIST endpoint to query Postgres basic (#3681) * reopening PR * PR feedback * small test fix * small PR feedbacks * adding new tests from match queries * PR feedback/formatting * temporary reversion to list endpoint for reconreport tests * reverting changes * adding logging * more logging * more logging * removing logging + commenting reconrep test temp * commenting out failing createReconReport spec * removing comment * reverting changes to reconReport test * reverting previous change * adding ts-check * PR feedback * PR feedback * adding in test * PR feedback fix * PR feedback * CUMULUS-3699 - Update collection List endpoints to query postgres - includeStats (#3688) * first commit * CHANGELOG * fixing small things * changes + fixes * PR feedback * splitting queries separately * PR feedback * PR feedback * PR feedback * CUMULUS-3639: Add support to db/CollectionSearch to retrieve active collections (#3693) * CUMULUS-3639:Add support to db/CollectionSearch to retrieve active collections * add test active collections * add ts-check * update /collections/active unit test * test snyk * fix field mapping * parallel search and fix urlPath * add cumulus-lp stack * add limit 1 to subquery * CUMULUS-3239 - Update Executions LIST endpoint to query Postgres basic (#3684) * first commit * adding changes * storing changes * updating progress * linting + small fixes * small fix * changing timestamp to string in tests * fixing timestamp * commenting out tests failing in CI but not locally * saving changes * collection support * adding async_ops support * changing endpoint + tests * fixing test * uncommenting tests + adding var * commenting out tests failing in CI but not locally * adding parentArn support + changing tests * added parentArn support + fixing tests * small endpoint test fix * Pr feedback + code improvements * small CHANGELOG fix * PR feedback * PR feedback + linting * PR feedback * PR feedback * fixing test * fixing execution tests after removing asyncCumulusOPId from mapping * PR feedback * removed includeFullRecord from search classes * PR feedback * PR feedback * reverting change * fix changelog * Cumulus 3640/3242- Update granule non-LIST endpoints and other granule related es queries to query postgres (#3727) * removing granules * fixing lint * fixing test * small change * adding back in some deleted things * removing more * lint fix * removing tests * skipping execution search-by-granules tests * skipping execution tests * removing tests * more removing * adding in deleted test * removing more * adding back in needed code * removing ES_HOST, query, and index from bulk_ops * fixing bulk_ops tests * adding back in ELK stack refs * changing reconreports test to skip/adding back in getGranulesByPayload * PR feedback * adding back in skipped tests * CHANGELOG * PR feedback * PR feedback' . ; * PR feedback + syntax check * adding back sort to write-granules test * CUMULUS-3240: Remove Elasticsearch dependency from executions endpoints (#3723) * CUMULUS-3240:Remove ElasticSearch dependency from Executions endpoints * update test-executions * remove es dependencies for execution * update changelog * fix lint and warning * address PR feedback * remove esClient from createExecutionRecords * CUMULUS-3642: postgres query adjustment (#3731) * estimate table row count * add more indexes * use count * * fix test * fix test * fix lint and add file index * add more index * fix lint * fix lint update test * execution asyncOperationId is optional * defautl stats last day * fix granule patchByGranuleId put logic back * vaccum tables * update changelog * remove sql script to another pr * update changelog * update active collection query * Revert "update active collection query" This reverts commit 88024c2. * CUMULUS-3238: Remove ElasticSearch dependency from Collection POST, PUT, and DEL endpoints (#3746) * first commit * CHANGELOG change * PR feedback * PR feedback * CUMULUS-3792: Add db indexes to improve search performance (#3751) * CUMULUS-3792:Add table indexes to improve search performance * fix changelog --------- Co-authored-by: Naga Nages <66387215+Nnaga1@users.noreply.github.com> Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>

estimate table row count

da28d2c

jennyhliu changed the base branch from master to feature/es-phase-1 July 16, 2024 20:17

jennyhliu and others added 17 commits July 17, 2024 11:42

add more indexes

ee54c29

use count *

1a3cf28

fix test

76f474f

Merge branch 'feature/es-phase-1' into CUMULUS-3642

fbca75e

fix test

7d51f58

fix lint and add file index

4274506

Merge branch 'feature/es-phase-1' into CUMULUS-3642

e843cb7

add more index

cb7877a

fix lint

29270db

fix lint update test

a6dbb31

execution asyncOperationId is optional

040dab5

defautl stats last day

90f3c8a

fix granule patchByGranuleId put logic back

ec50943

vaccum tables

8431406

update changelog

3361d85

remove sql script to another pr

23445b8

update changelog

ec9df7b

jennyhliu commented Jul 29, 2024

View reviewed changes

packages/api/endpoints/granules.js Show resolved Hide resolved

packages/api/endpoints/granules.js Show resolved Hide resolved

jennyhliu added the needs review label Jul 29, 2024

jennyhliu marked this pull request as ready for review July 29, 2024 17:33

jennyhliu changed the title ~~CUMULUS-3642: query adjustment~~ CUMULUS-3642: postgres query adjustment Jul 29, 2024

jennyhliu added 2 commits July 30, 2024 08:43

update active collection query

88024c2

Revert "update active collection query"

750b234

This reverts commit 88024c2.

Nnaga1 reviewed Jul 30, 2024

View reviewed changes

packages/db/src/search/BaseSearch.ts Show resolved Hide resolved

Nnaga1 reviewed Jul 30, 2024

View reviewed changes

packages/db/src/search/CollectionSearch.ts Show resolved Hide resolved

Nnaga1 reviewed Jul 30, 2024

View reviewed changes

packages/db/src/search/ExecutionSearch.ts Show resolved Hide resolved

Nnaga1 reviewed Jul 30, 2024

View reviewed changes

Merge branch 'feature/es-phase-1' into CUMULUS-3642

4e75e8c

Nnaga1 removed the needs review label Jul 31, 2024

Nnaga1 approved these changes Jul 31, 2024

View reviewed changes

Nnaga1 reviewed Jul 31, 2024

View reviewed changes

jennyhliu merged commit 867b325 into feature/es-phase-1 Jul 31, 2024
3 checks passed

jennyhliu deleted the CUMULUS-3642 branch July 31, 2024 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CUMULUS-3642: postgres query adjustment #3731

CUMULUS-3642: postgres query adjustment #3731

jennyhliu commented Jul 16, 2024 •

edited

Loading

Nnaga1 Jul 30, 2024

jennyhliu Jul 30, 2024

Nnaga1 Jul 30, 2024

Nnaga1 Jul 30, 2024

jennyhliu Jul 30, 2024

Nnaga1 Jul 30, 2024

jennyhliu Jul 30, 2024 •

edited

Loading

Nnaga1 Jul 30, 2024

jennyhliu Jul 30, 2024

Nnaga1 left a comment

Nnaga1 Jul 31, 2024

CUMULUS-3642: postgres query adjustment #3731

CUMULUS-3642: postgres query adjustment #3731

Conversation

jennyhliu commented Jul 16, 2024 • edited Loading

Changes

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennyhliu Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nnaga1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennyhliu commented Jul 16, 2024 •

edited

Loading

jennyhliu Jul 30, 2024 •

edited

Loading