-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[DBMON-6076] fix collect_wal_metrics default on 9.6 and below #22377
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
base: master
Are you sure you want to change the base?
Conversation
| # ERROR: Function pg_stat_get_wal_receiver() is currently not supported in Aurora | ||
| if self.is_aurora is False: | ||
| queries.append(QUERY_PG_STAT_WAL_RECEIVER) | ||
| if self._config.collect_wal_metrics is not False: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional sanity check
>>> # Default test
>>> None is not False
True
>>> # Explicitly enabled
>>> True is not False
True
>>> # Explicitly disabled
>>> False is not False
False| self.metadata_samples.run_job_loop(tags) | ||
| if self._config.collect_wal_metrics: | ||
| # collect wal metrics for pg < 10, disabled by enabled | ||
| if self._config.collect_wal_metrics is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional sanity check
>>> # Deafult test
>>> None is True
False
>>> # Explicitly enabled
>>> True is True
True
>>> # Explicitly disabled
>>> False is True
False
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
|
What does this PR do?
This fixes a regression introduced in #21347 where
collect_wal_metricswas accidentally changed to true by default for all Postgres versions. This is safe and desired for Postgres 10+ as we pull wal stats metrics frompg_ls_waldir. On Postgres 9.6 and below this function view doesn't exist and we need to pull wal stats directly from the local filesystem. This method doesn't work on older Postgres instances unless the agent is running on the same physical host and has the appropriate permissions to read the directory.Motivation
We're now fixing
collect_wal_metricsto work as it did previously where it defaults to unset and it will be implied enabled on Postgres 10+ unless explicitly disabled and implied disabled on Postgres 9.6 and below unless explicitly enabled. This is to provide a more stable out of the box experience.Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged