Thread: [HACKERS] Exclude pg_internal.init from base backup
Hackers, The cache in pg_internal.init was reused in days of yore but has been rebuilt on postmaster startup since v8.1. It appears there is no reason for this file to be backed up. I also moved the RELCACHE_INIT_FILENAME constant to relcache.h to avoid duplicating the string. I'll add this to the 2017-11 CF. Thanks, -- -David david@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Hi, On 02/09/17 21:08, David Steele wrote: > Hackers, > > The cache in pg_internal.init was reused in days of yore but has been > rebuilt on postmaster startup since v8.1. It appears there is no reason > for this file to be backed up. > Makes sense. > I also moved the RELCACHE_INIT_FILENAME constant to relcache.h to avoid > duplicating the string. +1 > +++ b/doc/src/sgml/protocol.sgml > @@ -2384,6 +2384,11 @@ The commands accepted in walsender mode are: > </listitem> > <listitem> > <para> > + <filename>pg_internal.init</> > + </para> > + </listitem> > + <listitem> > + <para> Not specific problem to this patch, but I wonder if it should be made more clear that those files (there are couple above of what you added) are skipped no matter which directory they reside in. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote: > Not specific problem to this patch, but I wonder if it should be made > more clear that those files (there are couple above of what you added) > are skipped no matter which directory they reside in. Agreed, it is a good idea to tell in the docs how this behaves. We could always change things so as the comparison is based on the full path like what is done for pg_control, but that does not seem worth complicating the code. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
> Not specific problem to this patch, but I wonder if it should be made
> more clear that those files (there are couple above of what you added)
> are skipped no matter which directory they reside in.
Agreed, it is a good idea to tell in the docs how this behaves. We
could always change things so as the comparison is based on the full
path like what is done for pg_control, but that does not seem worth
complicating the code.
pg_internal.init can, and do, appear in multiple different directories. pg_control is always in the same place. So they're not the same thing.
It also seems the list in the documentation is not in sync with the code. AFAICT docs are not mentioning the current_logfile. This seems to be a miss in 19dc233c32f ?
--
On 5 November 2017 at 11:55, Magnus Hagander <magnus@hagander.net> wrote: > On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier <michael.paquier@gmail.com> > wrote: >> >> On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek >> <petr.jelinek@2ndquadrant.com> wrote: >> > Not specific problem to this patch, but I wonder if it should be made >> > more clear that those files (there are couple above of what you added) >> > are skipped no matter which directory they reside in. >> >> Agreed, it is a good idea to tell in the docs how this behaves. We >> could always change things so as the comparison is based on the full >> path like what is done for pg_control, but that does not seem worth >> complicating the code. > > > pg_internal.init can, and do, appear in multiple different directories. > pg_control is always in the same place. So they're not the same thing. > > So +1 for documenting the difference in how these are handled, as this is > important to know for somebody writing an external tool for it. Changes made, moving to commit the attached patch. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On 11/7/17 11:03 AM, Simon Riggs wrote: > On 5 November 2017 at 11:55, Magnus Hagander <magnus@hagander.net> wrote: >> >> So +1 for documenting the difference in how these are handled, as this is >> important to know for somebody writing an external tool for it. > > Changes made, moving to commit the attached patch. Thanks, Simon! This was on my to do list today -- glad I checked my email first. -- -David david@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 8, 2017 at 1:42 AM, David Steele <david@pgmasters.net> wrote: > On 11/7/17 11:03 AM, Simon Riggs wrote: >> On 5 November 2017 at 11:55, Magnus Hagander <magnus@hagander.net> wrote: >>> >>> So +1 for documenting the difference in how these are handled, as this is >>> important to know for somebody writing an external tool for it. >> >> Changes made, moving to commit the attached patch. > > Thanks, Simon! This was on my to do list today -- glad I checked my > email first. <para> + <filename>pg_internal.init</filename> files can be omitted from the + backup whenever a file of that name is found. These files contain + relation cache data that is always rebuilt when recovering. + </para> Do we want to mention in the docs that the same decision-making is done for *all* files with matching names, aka the fact that if a file is listed and found in a sub-folder it is skipped? postmaster.opts or similar friends are unlikely to be found in anything but the root of the data folder, still the upthread argument of documenting precisely what basebackup.c does sounded rather convincing to me. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 8, 2017 at 3:03 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 5 November 2017 at 11:55, Magnus Hagander <magnus@hagander.net> wrote:
> On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
>> <petr.jelinek@2ndquadrant.com> wrote:
>> > Not specific problem to this patch, but I wonder if it should be made
>> > more clear that those files (there are couple above of what you added)
>> > are skipped no matter which directory they reside in.
>>
>> Agreed, it is a good idea to tell in the docs how this behaves. We
>> could always change things so as the comparison is based on the full
>> path like what is done for pg_control, but that does not seem worth
>> complicating the code.
>
>
> pg_internal.init can, and do, appear in multiple different directories.
> pg_control is always in the same place. So they're not the same thing.
>
> So +1 for documenting the difference in how these are handled, as this is
> important to know for somebody writing an external tool for it.
Changes made, moving to commit the attached patch.
The commit 98267e missed to check the empty SGML tag, attached patch
fixes the same.
Regards,
Hari Babu
Fujitsu Australia
Attachment
On Wed, Nov 8, 2017 at 9:04 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> The commit 98267e missed to check the empty SGML tag, attached patch
> fixes the same.
<listitem> <para>
- <filename>pg_internal.init (found in multiple directories)</>
+ <filename>pg_internal.init (found in multiple directories)</filename> </para> </listitem>
What has been committed in 98267ee and what is proposed here both look
incorrect to me. The markup filename ought to be used only with file
names, so "(found in multiple directories)" should not be within it.
Simon's commit is not wrong with the markup usage by the way.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 8, 2017 at 11:11 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Wed, Nov 8, 2017 at 9:04 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> The commit 98267e missed to check the empty SGML tag, attached patch
> fixes the same.
<listitem>
<para>
- <filename>pg_internal.init (found in multiple directories)</>
+ <filename>pg_internal.init (found in multiple directories)</filename>
</para>
</listitem>
What has been committed in 98267ee and what is proposed here both look
incorrect to me. The markup filename ought to be used only with file
names, so "(found in multiple directories)" should not be within it.
Simon's commit is not wrong with the markup usage by the way.
Thanks for the correction. I was not much aware of SGML markup usage.
While building the documentation, it raises an warning message of "empty end-tag".
So I just added the end tag. Attached the update patch with the suggested correction.
Regards,
Hari Babu
Fujitsu Australia
Attachment
On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote: > Thanks for the correction. I was not much aware of SGML markup usage. > While building the documentation, it raises an warning message of "empty > end-tag". > So I just added the end tag. Attached the update patch with the suggested > correction. Ah, I can see the warning as well. Using empty tags is forbidden since c29c5789, which is really recent. Sorry for missing it. Simon got trapped by that as well visibly. Your patch looks good to me. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 11/7/17 19:58, Michael Paquier wrote: > On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote: >> Thanks for the correction. I was not much aware of SGML markup usage. >> While building the documentation, it raises an warning message of "empty >> end-tag". >> So I just added the end tag. Attached the update patch with the suggested >> correction. > > Ah, I can see the warning as well. Using empty tags is forbidden since > c29c5789, which is really recent. Sorry for missing it. Simon got > trapped by that as well visibly. Your patch looks good to me. fixed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 9, 2017 at 1:03 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 11/7/17 19:58, Michael Paquier wrote: >> On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote: >>> Thanks for the correction. I was not much aware of SGML markup usage. >>> While building the documentation, it raises an warning message of "empty >>> end-tag". >>> So I just added the end tag. Attached the update patch with the suggested >>> correction. >> >> Ah, I can see the warning as well. Using empty tags is forbidden since >> c29c5789, which is really recent. Sorry for missing it. Simon got >> trapped by that as well visibly. Your patch looks good to me. > > fixed Thanks. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers