Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite Postgres size query and add postgresql.relation.{tuples,pages,all_visible} + toast_size metrics #14500

Merged
merged 6 commits into from
Jun 16, 2023

Conversation

bonnefoa
Copy link
Contributor

@bonnefoa bonnefoa commented May 3, 2023

What does this PR do?

  • Change metric: postgresql.table_size now only reports the size of the main table (removing toast size, fsm and vm)
  • Report postgresql.toast_size as table_size + index_size + toast_size.
  • New metric: postgresql.toast_size reports the size of the associated toast table
  • Optimise the query to lower the amount of stat calls
  • Add a tag_none type which doesn't report the tag if the value is none
  • Add a partition_of tag if the table is a partition

Motivation

Size metrics needs to call admin function pg_table_size/pg_indexes_size/pg_total_relation_size. Those functions will generate multiple stat syscall behind the hood (see dbsize.c) to get the size of the files on disk which can be expensive with a high number of relation.
We also want to keep the precision of how much of the table is stored in toast table or not.
We manually compute the total to avoid the call to pg_total_relation_size, this will effectively divide the number of stat calls by 2.
We rely on pg_relation_size instead of pg_table_size. pg_relation_size don't include the toast so we can generate toast size and table size in different metric. We lose the size of FSM and VM but their sizes are generally dwarfed by the main table and it's a good trade off to skip those to save stat calls.
pg_indexes_size and pg_relation_size calls still need to lock the relation. We know from relhasindex and reltoastrelid if a relation has indexes or a toast table so we can only call the functions only when necessary.

tag_none has been created to only report the tag partition_of when the table is a partition. Normal tables should not report the tag at all.

Benchmarks and timings

Old query

explain analyze SELECT
  N.nspname,
  relname,
  pg_table_size(C.oid) as table_size,
  pg_indexes_size(C.oid) as index_size,
  pg_total_relation_size(C.oid) as total_size
FROM pg_class C
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
WHERE nspname NOT IN ('pg_catalog', 'information_schema') AND
  nspname !~ '^pg_toast' AND
  relkind = 'r';
                                                       QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------
 Hash Join  (cost=7.26..1521.93 rows=284 width=152) (actual time=0.214..23.036 rows=325 loops=1)
   Hash Cond: (c.relnamespace = n.oid)
   ->  Seq Scan on pg_class c  (cost=0.00..1511.11 rows=391 width=72) (actual time=0.009..1.365 rows=391 loops=1)
         Filter: (relkind = 'r'::"char")
         Rows Removed by Filter: 1940
   ->  Hash  (cost=7.17..7.17 rows=8 width=68) (actual time=0.025..0.025 rows=8 loops=1)
         Buckets: 1024  Batches: 1  Memory Usage: 9kB
         ->  Seq Scan on pg_namespace n  (cost=0.00..7.17 rows=8 width=68) (actual time=0.008..0.021 rows=8 loops=1)
               Filter: ((nspname <> ALL ('{pg_catalog,information_schema}'::name[])) AND (nspname !~ '^pg_toast'::text))
               Rows Removed by Filter: 3
 Planning Time: 0.151 ms
 Execution Time: 23.081 ms

stat calls

strace -e trace=newfstatat -p $(pgrep -f "postgres.*idle") -c
strace: Process 62163 attached
^Cstrace: Process 62163 detached
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
  0.00    0.000000           0        86        56 newfstatat
------ ----------- ----------- --------- --------- ----------------
100.00    0.000000           0        86        56 total

New query

 explain analyze SELECT current_database(),
       s.schemaname, s.table, s.parent_table_name,
       s.relpages, s.reltuples, s.relallvisible,
       s.table_size, s.index_size, s.toast_size, s.table_size + s.index_size + s.toast_size
FROM
    (SELECT
      N.nspname as schemaname,
      relname as table,
      I.inhparent::regclass AS parent_table_name,
      C.relpages,
      C.reltuples,
      C.relallvisible,
      pg_relation_size(C.oid) as table_size,
      CASE WHEN C.relhasindex THEN pg_indexes_size(C.oid) ELSE 0 END as index_size,
      CASE WHEN C.reltoastrelid > 0 THEN pg_relation_size(C.reltoastrelid) ELSE 0 END as toast_size
    FROM pg_class C
    LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
    LEFT JOIN pg_inherits I ON (I.inhrelid = C.oid)
    WHERE NOT (nspname = ANY('{pg_catalog,information_schema}')) AND
      relkind = 'r') as s;
                                                             QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------
 Subquery Scan on s  (cost=1523.25..1569.24 rows=320 width=240) (actual time=1.446..10.677 rows=325 loops=1)
   ->  Hash Join  (cost=1523.25..1563.64 rows=320 width=168) (actual time=1.444..10.563 rows=325 loops=1)
         Hash Cond: (c.relnamespace = n.oid)
         ->  Hash Right Join  (cost=1516.00..1551.77 rows=391 width=93) (actual time=1.366..1.454 rows=391 loops=1)
               Hash Cond: (i.inhrelid = c.oid)
               ->  Seq Scan on pg_inherits i  (cost=0.00..30.40 rows=2040 width=8) (actual time=0.002..0.002 rows=0 loops=1)
               ->  Hash  (cost=1511.11..1511.11 rows=391 width=89) (actual time=1.360..1.361 rows=391 loops=1)
                     Buckets: 1024  Batches: 1  Memory Usage: 56kB
                     ->  Seq Scan on pg_class c  (cost=0.00..1511.11 rows=391 width=89) (actual time=0.007..1.284 rows=391 loops=1)
                           Filter: (relkind = 'r'::"char")
                           Rows Removed by Filter: 1940
         ->  Hash  (cost=7.14..7.14 rows=9 width=68) (actual time=0.020..0.021 rows=9 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 9kB
               ->  Seq Scan on pg_namespace n  (cost=0.00..7.14 rows=9 width=68) (actual time=0.007..0.017 rows=9 loops=1)
                     Filter: (nspname <> ALL ('{pg_catalog,information_schema}'::name[]))
                     Rows Removed by Filter: 2
 Planning Time: 0.239 ms
 Execution Time: 10.730 ms
(18 rows)

Stat calls

strace -e trace=newfstatat -p $(pgrep -f "postgres.*idle") -c
strace: Process 62163 attached
^Cstrace: Process 62163 detached
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
  0.00    0.000000           0        23        16 newfstatat
------ ----------- ----------- --------- --------- ----------------
100.00    0.000000           0        23        16 total

We went from 86 stat calls to 23

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@codecov
Copy link

codecov bot commented May 3, 2023

Codecov Report

Merging #14500 (f780d20) into master (4e078b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
postgres 91.33% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

github-actions bot commented May 3, 2023

Test Results

     12 files       12 suites   11m 45s ⏱️
   210 tests    208 ✔️   2 💤 0
1 266 runs  1 222 ✔️ 44 💤 0

Results for commit f780d20.

♻️ This comment has been updated with latest results.

@justiniso
Copy link
Contributor

For my education, the flag -f "postgres.*idle", why is idle in there?

@bonnefoa
Copy link
Contributor Author

bonnefoa commented May 5, 2023

For my education, the flag -f "postgres.*idle", why is idle in there?

I wanted to look at the backend process matching the psql session which will run the query I'm testing.
If you check all processes with a postgres's name, you're gonna match all postgres's processes + psql backend process

pgrep -a -f "postgres:"
1017179 postgres: checkpointer
1017180 postgres: background writer
1017182 postgres: walwriter
1017183 postgres: autovacuum launcher
1017184 postgres: logical replication launcher
1826329 postgres: postgres postgres [local] idle

An idle session will be tagged as idle in the process name so the "postgres.*idle" allowed me to match the backend process of the psql session I have opened.

pgrep -a -f "postgres.*idle"
1826329 postgres: postgres postgres [local] idle

@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch 2 times, most recently from 86a05a6 to 93cdb34 Compare May 5, 2023 13:36
@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch 14 times, most recently from 9a682c0 to 99b01d0 Compare May 11, 2023 07:26
@JordanP
Copy link
Contributor

JordanP commented May 15, 2023

@bonnefoa thanks for doing this. Is this going to superseed #14156 and #14056 ? (cc @FlipEnergy)

@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch 2 times, most recently from 48094d9 to f863784 Compare June 8, 2023 14:07
@bonnefoa
Copy link
Contributor Author

bonnefoa commented Jun 8, 2023

Here are perf before and after the patch
old_perf
new_perf
Time spent InterpExpr went from 87% to 56%

@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch 2 times, most recently from d5462c6 to b9b151a Compare June 15, 2023 15:34
@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch 3 times, most recently from e30dd66 to b7af4c2 Compare June 15, 2023 15:48
bonnefoa added 6 commits June 16, 2023 09:45
Use new query executor
Split toast size from table size
Add partition_of tag
Add pages/tuples/allvisible metrics
Optimise query to minimise stat calls
@bonnefoa bonnefoa force-pushed the bonnefoa/pg-size-table branch from b7af4c2 to f780d20 Compare June 16, 2023 07:50
Copy link
Contributor

@alexandre-normand alexandre-normand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @bonnefoa . Nice detailed analysis of the impact of the changes, too.

@alexandre-normand alexandre-normand merged commit f63aa87 into master Jun 16, 2023
@alexandre-normand alexandre-normand deleted the bonnefoa/pg-size-table branch June 16, 2023 19:44
aquiladayc added a commit that referenced this pull request Jun 28, 2023
remove lines

Update CHANGELOG.md

Draft CHANGELOG.md

Update metadata.csv

Update metadata.json

Update conftest.py

Update CHANGELOG.md

Remove change log

Update ecs_fargate.py

Add __init__ function

Fix ecs_fargate.py and add tests

Add new performance counter metrics (#14625)

* Add new cluster metrics

* Add host and VM metrics

* Add realtime fixtures

* update metadata.csv

* Add throughput metric

Move cancel waiting logic to test functions for DBMAsyncJob  (#14773)

* init commit

* revert changes to sqlserver, mysql, utils tests

Add validations for removed dependencies (#14556)

* Map out new licenses validation

* Implement validations for extra licenses

* Add constants to config.toml

* Implement license validation

* Uncomment legacy licenses validation

* Keep license command addition in same place

* Small style change

* Update config.toml override values

* Refactor

* Fix style

* Apply suggestions from code review

Co-authored-by: Ofek Lev <[email protected]>

* Update suggestions

* Require CI for license validation tests and update to use empty envvars

* Fix permission for file

* Add windows version of setting github env vars

* Fix windows file

* Change to powershell script

* Output GITHUB_ENV on windows CI

* Convert entirely to powershell

* Change back to bat file

* Test DD_GITHUB_USER value

* Print github user in license test

* Manually set Github user and token in test

* Fix config_file

* Print github user

* Check if tokens are the same

* Remove additional space in bat script

* Fix style and remove test code

* Change order of scripts

* Try commenting out model.github override

* Revert previous commit

* Change to threads instead of async

* Switch out async request to requests

* Clean up

* Fix style

---------

Co-authored-by: Ofek Lev <[email protected]>

DOCS-5656 gke setup links to operator/helm (#14746)

dbm-oracle-dashboard (#14736)

Internally compile the `include` patterns in the autodiscovery feature (#14768)

* Internally compile the `include` patterns in the discovery feature

* address

Use Git for versioning (#14778)

Upgrade Pydantic model code generator (#14779)

* Upgrade Pydantic model code generator

* address

build standalone binaries for ddev (#14774)

Remove `pyperclip` dependency and clipboard functionality (#14782)

[Release] Bumped datadog_checks_dev version to 20.0.0 (#14784)

* [Release] Bumped datadog_checks_dev version to 20.0.0

* [Release] Update metadata

Bump the minimum version of datadog-checks-dev (#14785)

update license path (#14783)

Allow all projects to be collected in REST implementation (#14433)

* Bug Fix Teamcity rest with all projects

* .

Rewrite Postgres size query and add `postgresql.relation.{tuples,pages,all_visible}` + toast_size metrics (#14500)

* Bumped dependency version

* Rewrite size metric query

Use new query executor
Split toast size from table size
Add partition_of tag
Add pages/tuples/allvisible metrics
Optimise query to minimise stat calls

* Add partitioned test tables

* Update metadata with new metric

* Fix version check

* Fix wal_level for tests

[SNMP] Add metadata for traps telemetry metrics (#14769)

* Add metadata for traps telemetry metrics

* Remove commas from desc 🤦‍

* Add units

Temporarily disable py2 tests on PRs (#14793)

fix(redisdb): return len of stream instead of 1 (#14722)

Currently the code compute the len of the stream but report always 1

This change fixes this.

Update Netflow dashboard (#14794)

* update Netflow dashboard

* remove datadog_demo_keep:true

* avg -> sum

* rename dashboard

revert manifest.json (#14797)

Add User Profiles support (#14752)

Remove Content (#14766)

Update wording and add extra install directions for ODBC (#14781)

* Update wording and add extra install directions for ODBC

* Update README.md

Add ability to choose tag to append to VM hostname (#14657)

* Add ability to choose tag to append to VM hostname

* Add a test for integration tags

* Sort list

* Change log to debug

* Fix style

* Allow user to choose a datadog tag for vm hostname

Fix ability to release ddev (#14790)

Disable server info and version collection when collect_server_info is false (#14610)

* if collect_server_info is set to false disable server info and version collection

* the collect metadata in check.py has to check for the collect_server_info before attempting to collect server info, even when the base url is well formated

* add testing to see if metadata is collected when collect_server_info is false

* add testing to see if metadata is collected when collect_server_info is false

* fix typo

* fix typo

* commit

* commit

* commit

* commit typo

* fix check.py

[Release] Bumped datadog_checks_dev version to 20.0.1 (#14806)

* [Release] Bumped datadog_checks_dev version to 20.0.1

* [Release] Update metadata

[Release] Bumped ddev version to 3.0.0 (#14807)

* [Release] Bumped ddev version to 3.0.0

* [Release] Update metadata

fix build flake for ddev (#14808)

Fix ddev platform installers and releasing (#14812)

Bump postgres integration to Python 3 (#14813)

update changelog generation (#14810)

Update ecs_fargate/tests/fixtures/metadata_v4.json

Co-authored-by: Cedric Lamoriniere <[email protected]>

Update ecs_fargate/tests/fixtures/stats_linux_v4.json

Co-authored-by: Cedric Lamoriniere <[email protected]>

Update test_unit_v4.py

Update license term

Update test_unit_v4.py

update license format

Update metadata.csv

Fix unit name

Set the `marker` option to `not e2e` by default (#14804)

Add profile for hp-ilo (#14771)

* add profile for hp-ilo

* add tests for new hp-ilo profile

* fix linter

* hp-ilo4 extends hp-ilo

* delete unnecessary product_name field

* lint

* move hp-ilo to default-profiles

Update profiles with missing devices (#14695)

* update cisco-asr

* update cisco-catalyst-wlc

* update cisco-catalyst

* update cisco-legacy-wlc

* update cisco-nexus

* update dell-poweredge

* update juniper-ex

* update juniper-mx

* add cisco-isr

* add models + move cisco5700WLC to cisco-catalyst-wlc

* move cisco-isr to default-profiles

Add profile 3com-huawei (#14694)

Revert "Set the `marker` option to `not e2e` by default (#14804)" (#14815)

This reverts commit 3f4c885.

Sort assert_device_metadata tags (#14816)

Add per vendor generic profiles (#14721)

* add dell generic profile

* add fortinet generic profile

* add juniper generic profile

* move vendor profiles to default-profiles

* add test for cisco

* add test for dell

* add test for fortinet

* add test for juniper

* linter

* linter

Update formatting for changelogs (#14814)

* Update formatting for changelogs

* Update formatting for changelogs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants