| From: | David Steele <david(at)pgmasters(dot)net> |
|---|---|
| To: | Magnus Hagander <magnus(at)hagander(dot)net>, Marco Nenciarini <marco(dot)nenciarini(at)2ndquadrant(dot)it> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Updated backup APIs for non-exclusive backups |
| Date: | 2016-04-04 12:40:19 |
| Message-ID: | 570260B3.6090504@pgmasters.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 3/19/16 8:15 AM, Magnus Hagander wrote:
> I've attached an updated patch, which is rebased on current master and
> includes the oid fix.
I've now had a chance to go through this in detail and test thoroughly.
We had a lot of discussion about copying pg_control last and that led
me to believe that there might be some sort of race condition with
multiple backups running concurrently.
I tried every trick I could think of and everything worked as expected
so as far as I can see this requirement only applies to backups taken
from a standby which agrees with the comments in do_pg_stop_backup().
Note that I only tested backups against a master database since that's
what is modified by this patch. I'm still not entirely comfortable with
how backups against a standby are done and it that case still think it
would be best to write the stop wal location into backup_label, but as
you said before that would be a completely separate patch.
So in the end I'm fine with the code as it stands with the exception of
the pg_stop_backup2() naming. I'd prefer a more verbose name here but
I'm unable to come up with anything very good. How about
pg_stop_backup_v2()? This function could also use a few more comments,
at least at the top of the exclusive and non-exclusive blocks.
Except for those minor details I believe this is "ready for committer"
unless anybody has an objection.
--
-David
david(at)pgmasters(dot)net
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Aleksander Alekseev | 2016-04-04 12:56:56 | Tiny patch: sigmask.diff |
| Previous Message | Tomas Vondra | 2016-04-04 12:25:11 | Re: GIN data corruption bug(s) in 9.6devel |