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

feat: add metrics for pg_stat_checkpointer in v17+ #4341

Closed
wants to merge 3 commits into from
Closed

Conversation

NiccoloFei
Copy link
Collaborator

Closes #3411

@github-actions github-actions bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.21 release-1.22 labels Apr 19, 2024
Copy link
Contributor

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@gbartolini
Copy link
Contributor

IMHO, this patch should have first fixed the issue for 17 then added pg_stat_checkpointer.

@NiccoloFei
Copy link
Collaborator Author

NiccoloFei commented Apr 29, 2024

Because the query mapping needs to be unique (even though the 2 queries are mutually exclusive), I've distinguished the bgwriter metric's name in the following way:

  • for PG17 and onward: pg_stat_background_writer
  • for PG16 and lower: pg_stat_bgwriter (retaining the same name)

I've also thought about including and serving the 4 remaining bgwriter columns directly from the pg_stat_checkpointer query (on PG17+) but I figured it would be even more confusing this way, so I opted for the previous solution.

Tests running on: https://github.com/cloudnative-pg/postgres-trunk-containers/actions/runs/8881859306

@sxd
Copy link
Member

sxd commented May 3, 2024

/test depth=push limit=local test_level=4 feature_type=observability

Copy link
Contributor

github-actions bot commented May 3, 2024

@sxd, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/8939199640

@github-actions github-actions bot added the ok to merge 👌 This PR can be merged label May 3, 2024
NiccoloFei and others added 3 commits May 7, 2024 16:44
Signed-off-by: Niccolò Fei <niccolo.fei@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Niccolò Fei <niccolo.fei@enterprisedb.com>
@fcanovai
Copy link
Contributor

I don't like the idea to generate metrics with different names from the same table, although on different postgres version.

Additionally, such changes should affect the grafana dashboard as well.

@mnencia
Copy link
Member

mnencia commented Jun 7, 2024

Superseded by #4779

@mnencia mnencia closed this Jun 7, 2024
@mnencia mnencia deleted the dev/3411 branch June 7, 2024 14:20
Copy link
Contributor

github-actions bot commented Jun 7, 2024

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-requested ◀️ This pull request should be backported to all supported releases ok to merge 👌 This PR can be merged release-1.21 release-1.22 release-1.23
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: PostgreSQL 17 removed some columns from pg_stat_bgwriter and introduced pg_stat_checkpointer
5 participants