On Sat, Jul 30, 2022 at 5:37 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Tue, Jul 26, 2022 at 5:50 PM David Steele <da...@pgmasters.net> wrote: > > > > >> I would prefer to have all the components of backup_label stored > > >> separately and then generate backup_label from them in pg_backup_stop(). > > > > > > +1, because pg_backup_stop is the one that's returning backup_label > > > contents, so it does make sense for it to prepare it once and for all > > > and return. > > > > > >> For PG16 I am planning to add some fields to backup_label that are not > > >> known when pg_backup_start() is called, e.g. min recovery time. > > > > > > Can you please point to your patch that does above? > > > > Currently it is a plan, not a patch. So there is nothing to show yet. > > > > > Yes, right now, backup_label or tablespace_map contents are being > > > filled in by pg_backup_start and are never changed again. But if your > > > above proposal is for fixing some issue, then it would make sense for > > > us to carry all the info in a structure to pg_backup_stop and then let > > > it prepare the backup_label and tablespace_map contents. > > > > I think this makes sense even if I don't get these changes into PG16. > > > > > If the approach is okay for the hackers, I would like to spend time on it. > > > > +1 from me. > > Here comes the v1 patch. This patch tries to refactor backup related > code, advantages of doing so are following: > > 1) backup state is more structured now - all in a single structure, > callers can create backup_label contents whenever required, either > during the pg_backup_start or the pg_backup_stop or in between. > 2) no parsing of backup_label file lines now in pg_backup_stop, no > error checking for invalid parsing. > 3) backup_label and history file contents have most of the things in > common, they can now be created within a single function. > 4) makes backup related code extensible and readable. > > One downside is that it creates a lot of diff with previous versions. > > Please review.
I added this to current CF - https://commitfest.postgresql.org/39/3808/ -- Bharath Rupireddy RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/
