Skip to content

plugins: postgresql add support for PostgreSQL 17 + improvements#2004

Merged
BareosBot merged 4 commits intobareos:masterfrom
bruno-at-bareos:dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17
Dec 11, 2024
Merged

plugins: postgresql add support for PostgreSQL 17 + improvements#2004
BareosBot merged 4 commits intobareos:masterfrom
bruno-at-bareos:dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17

Conversation

@bruno-at-bareos
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos commented Nov 6, 2024

This PR aims to fix issue #1982 problem with incremental on PG17

  • add support for PostgreSQL version 17, we now use dedicated function
    pg_walfile_name_offset to retrieve filename and check if offset is 0.
  • database_local: set checkpoint timeout lower.
  • add support for nanosecond integer resolution.
    • use ns in self.last_backup_stop_time as datetime object are not serializable.
    • use ns for start_backup_time and remove datetime usage for file comparison.
    • this support allow the plugin to not backup each time the latest wal file.
  • add fix for pg8000 < 1.30 returning a string instead tuple with pg_walfile_name_offset.
  • add a check if ROP last_time_backup is large enough or convert it to ns (ease plugin version migration).
  • removed unused modules datetime, dateutil
  • write a new ADR about using always integer in timestamp comparison in python code.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Required backport PRs have been created
  • Correct milestone is set
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR

@bruno-at-bareos bruno-at-bareos linked an issue Nov 7, 2024 that may be closed by this pull request
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from c967c29 to 6099c78 Compare November 11, 2024 14:30
@bruno-at-bareos bruno-at-bareos changed the title dev/master/bruno/fix issue#1982 postgresql plugin with pg17 plugins: postgresql add support for PostgreSQL 17 + improvements Nov 11, 2024
@bruno-at-bareos bruno-at-bareos self-assigned this Nov 11, 2024
@bruno-at-bareos bruno-at-bareos added this to the 25.0.0 milestone Nov 11, 2024
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from 6099c78 to eaf6af1 Compare November 11, 2024 14:36
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from 8131e61 to cc58878 Compare November 12, 2024 09:14
@bruno-at-bareos bruno-at-bareos marked this pull request as ready for review November 12, 2024 09:15
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from cc58878 to 2588105 Compare November 25, 2024 10:30
@arogge arogge requested a review from sebsura November 26, 2024 10:42
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from c040355 to f6a6b09 Compare November 28, 2024 09:34
@bruno-at-bareos bruno-at-bareos force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch 2 times, most recently from 2f3cbd1 to a248741 Compare December 3, 2024 12:34
@bruno-at-bareos
Copy link
Contributor Author

Hi Sebastian, this morning with the formal acceptance of the new ADR, I've renamed the file and squash the commit for a final review.
Have fun.

Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

There seems to be a sporadic test error with this PR:
See e.g. here: https://jenkins.bareos.com/blue/organizations/jenkins/bareos/detail/PR-2004/27/tests
It did not backup or restore recovery.signal correctly.

@bruno-at-bareos
Copy link
Contributor Author

There seems to be a sporadic test error with this PR: See e.g. here: https://jenkins.bareos.com/blue/organizations/jenkins/bareos/detail/PR-2004/27/tests It did not backup or restore recovery.signal correctly.

As we've seen some case where Jenkins didn't got all the files, we know it is temporary failing. If recovery.signal was missing the Restore job wouldn't be ok.

bruno-at-bareos and others added 4 commits December 11, 2024 19:10
Our plugin is able to work with PostgreSQL 10.

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
- add support for PostgreSQL version 17, we now use dedicated
  function pg_walfile_name_offset to retrieve filename and
  check if offset is 0.
- database_local: set checkpoint timeout lower.
- add support for nanosecond integer resolution.
  + use ns in self.last_backup_stop_time as datetime object
    are not serializable.
  + use ns for start_backup_time and remove datetime usage
    for file comparison.
  + this support allow the plugin to not backup each time the
    latest wal file.
- add fix for pg8000 < 1.30 returning a string instead tuple
  with pg_walfile_name_offset.
- add a check if ROP last_time_backup is large enough
  or convert it to ns (ease plugin version migration).
- remove unused modules datetime dateutil.
- use constant for `NANOSECONDS_PER_SECOND` and
  `LAST_BACKUP_TIME_WITH_SECONDS`
- limit if/else block by using global functions current_time_ns()
- add comment about time.time_ns() when to remove it (python 3.7 eol)
- use directly os.stat().st_mtime_ns it has been introduced in
  python 3.3 and we don't support python <= 3.6.
- add comment about checkpoint_timeout parameter in setup-local-db.
- use stat_obj instead of stat to avoid keyword redefining.
- improve docstring for __decode_lsn_filename_offset which return
  True.
- remove unused lsn parameter from __wait_for_wal_archiving.
- add debug message when LAST_BACKUP_TIME_WITH_SECONDS is hit.

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Co-authored-by: Sebastian Sura <124262655+sebsura@users.noreply.github.com>
Introduce newly accepted adr for python code:
0003-always-use-integer-when-comparing-timestamp-numbers-in-python

Fix typo in README.md

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Co-authored-by: Sebastian Sura <124262655+sebsura@users.noreply.github.com>
@BareosBot BareosBot force-pushed the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch from 912841e to 73dd9b8 Compare December 11, 2024 19:11
@BareosBot BareosBot merged commit 4a22055 into bareos:master Dec 11, 2024
@bruno-at-bareos bruno-at-bareos deleted the dev/master/bruno/fix-issue#1982-postgresql-plugin-with-pg17 branch December 24, 2024 10:43
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.

PostgreSQL 17 incremental backup issue (PostgreSQL plugin)

3 participants