Re: [HACKERS] Exclude pg_internal.init from base backup

[Michael Paquier]

On Thu, Nov 9, 2017 at 1:03 AM, Peter Eisentraut
 wrote:
> On 11/7/17 19:58, Michael Paquier wrote:
>> On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi  
>> 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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Peter Eisentraut]

On 11/7/17 19:58, Michael Paquier wrote:
> On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi  
> 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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Michael Paquier]

On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi  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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Haribabu Kommi]

On Wed, Nov 8, 2017 at 11:11 AM, Michael Paquier 
wrote:

> On Wed, Nov 8, 2017 at 9:04 AM, Haribabu Kommi 
> wrote:
> > The commit 98267e missed to check the empty SGML tag, attached patch
> > fixes the same.
>
> 
>  
> - pg_internal.init (found in multiple directories)
> + pg_internal.init (found in multiple
> directories)
>  
> 
> 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


sgml_empty_tag_fix_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Exclude pg_internal.init from base backup

[Michael Paquier]

On Wed, Nov 8, 2017 at 9:04 AM, Haribabu Kommi  wrote:
> The commit 98267e missed to check the empty SGML tag, attached patch
> fixes the same.


 
- pg_internal.init (found in multiple directories)
+ pg_internal.init (found in multiple directories)
 

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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Haribabu Kommi]

On Wed, Nov 8, 2017 at 3:03 AM, Simon Riggs  wrote:

> On 5 November 2017 at 11:55, Magnus Hagander  wrote:
> > On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier <
> michael.paqu...@gmail.com>
> > wrote:
> >>
> >> On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
> >>  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


sgml_empty_tag_fix.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Exclude pg_internal.init from base backup

[Michael Paquier]

On Wed, Nov 8, 2017 at 1:42 AM, David Steele  wrote:
> On 11/7/17 11:03 AM, Simon Riggs wrote:
>> On 5 November 2017 at 11:55, Magnus Hagander  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.


+pg_internal.init 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.
+   
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

Re: [HACKERS] Exclude pg_internal.init from base backup

[David Steele]

On 11/7/17 11:03 AM, Simon Riggs wrote:
> On 5 November 2017 at 11:55, Magnus Hagander  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
da...@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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Simon Riggs]

On 5 November 2017 at 11:55, Magnus Hagander  wrote:
> On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier 
> wrote:
>>
>> On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
>>  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 Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


pg_basebackup-exclusion-v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Exclude pg_internal.init from base backup

[Magnus Hagander]

On Sat, Nov 4, 2017 at 4:04 AM, Michael Paquier 
wrote:

> On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
>  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.

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 ?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Re: [HACKERS] Exclude pg_internal.init from base backup

[Michael Paquier]

On Fri, Nov 3, 2017 at 4:04 PM, Petr Jelinek
 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

Re: [HACKERS] Exclude pg_internal.init from base backup

[Petr Jelinek]

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:
> 
> 
>  
> + pg_internal.init
> +
> +   
> +   
> +

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