Re: Does recovery write to backup_label ?

2020-02-09 Thread David Steele




On 2/7/20 8:06 PM, Fujii Masao wrote:

On Sat, Feb 8, 2020 at 5:05 AM Chapman Flack  wrote:


On 2/7/20 2:55 PM, Andres Freund wrote:


If the file needs to have 0600 permissions, should there be
a note in the nonexclusive-mode backup docs to say so?


I'm not convinced that that's useful. The default is that everything
needs to be writable by postgres. The exceptions should be noted if
anything, not the default.


+1


+1.

In theory it would be more secure to only allow rename, but since 
Postgres can overwrite any other file in the cluster I don't see much 
value in making an exception for this file.



Could this arguably be a special case, as most things in the datadir
are put there by postgres, but the backup_label is now to be put there
(and not even 'there' there, but added as a final step only to a
'backup-copy-of-there' there) by the poor schmuck who reads the
non-exclusive backup docs as saying "retrieve this content from
pg_stop_backup() and preserve in a file named backup_label" and can't
think of any obvious reason to put write permission on a file
that preserves immutable history in a backup?


I have no strong objection to add more note about permissions
of the files that the users put in the data directory. If we really
do that, it'd be better to note about not only backup_label
but also other files like tablespace_map, recovery.signal,
promotion trigger file, etc.


More documentation seems like a good idea, especially since 
non-exclusive backup requires the app to choose/set permissions.


Regards,
--
-David
da...@pgmasters.net




Re: Does recovery write to backup_label ?

2020-02-07 Thread Fujii Masao
On Sat, Feb 8, 2020 at 5:05 AM Chapman Flack  wrote:
>
> On 2/7/20 2:55 PM, Andres Freund wrote:
>
> >> If the file needs to have 0600 permissions, should there be
> >> a note in the nonexclusive-mode backup docs to say so?
> >
> > I'm not convinced that that's useful. The default is that everything
> > needs to be writable by postgres. The exceptions should be noted if
> > anything, not the default.

+1

> Could this arguably be a special case, as most things in the datadir
> are put there by postgres, but the backup_label is now to be put there
> (and not even 'there' there, but added as a final step only to a
> 'backup-copy-of-there' there) by the poor schmuck who reads the
> non-exclusive backup docs as saying "retrieve this content from
> pg_stop_backup() and preserve in a file named backup_label" and can't
> think of any obvious reason to put write permission on a file
> that preserves immutable history in a backup?

I have no strong objection to add more note about permissions
of the files that the users put in the data directory. If we really
do that, it'd be better to note about not only backup_label
but also other files like tablespace_map, recovery.signal,
promotion trigger file, etc.

Regards,

-- 
Fujii Masao




Re: Does recovery write to backup_label ?

2020-02-07 Thread Chapman Flack
On 2/7/20 2:55 PM, Andres Freund wrote:

>> If the file needs to have 0600 permissions, should there be
>> a note in the nonexclusive-mode backup docs to say so?
> 
> I'm not convinced that that's useful. The default is that everything
> needs to be writable by postgres. The exceptions should be noted if
> anything, not the default.

Could this arguably be a special case, as most things in the datadir
are put there by postgres, but the backup_label is now to be put there
(and not even 'there' there, but added as a final step only to a
'backup-copy-of-there' there) by the poor schmuck who reads the
non-exclusive backup docs as saying "retrieve this content from
pg_stop_backup() and preserve in a file named backup_label" and can't
think of any obvious reason to put write permission on a file
that preserves immutable history in a backup?

Regards,
-Chap




Re: Does recovery write to backup_label ?

2020-02-07 Thread Andres Freund
Hi,

On 2020-02-07 11:08:48 -0500, Chapman Flack wrote:
> Just saw this in a PG 11.6 cluster starting a recovery:
> 
> 2020-02-07 10:45:40 EST FATAL:  42501: could not open file
> "backup_label": Permission denied
> 2020-02-07 10:45:40 EST LOCATION:  fsync_fname_ext, fd.c:3531

Well, we generally assume that files in the data directory are writable,
with a very few exceptions. And we do need to be able rename
backup_label to backup_label.old. Which strictly speaking doesn't
require write permissions on the file - but I assume that's what
triggers the issue here. There's IIRC platforms that don't like fsyncing
files with a O_RDONLY fd, so if we want to rename safely (which entails
fsyncing beforehand), we don't have much choice.


> I had assumed the label file would be treated as readonly
> during recovery.

It is IIRC documented that it does get renamed...

> If the file needs to have 0600 permissions, should there be
> a note in the nonexclusive-mode backup docs to say so?

I'm not convinced that that's useful. The default is that everything
needs to be writable by postgres. The exceptions should be noted if
anything, not the default.

Greetings,

Andres Freund




Does recovery write to backup_label ?

2020-02-07 Thread Chapman Flack
Just saw this in a PG 11.6 cluster starting a recovery:

2020-02-07 10:45:40 EST FATAL:  42501: could not open file
"backup_label": Permission denied
2020-02-07 10:45:40 EST LOCATION:  fsync_fname_ext, fd.c:3531

The label file was written with mode 0400 by a script that got
the contents from pg_stop_backup(boolean,boolean).

But during recovery, it is being poked at by fsync_fname_ext
which wants to open it O_RDWR.

I had assumed the label file would be treated as readonly
during recovery.

If the file needs to have 0600 permissions, should there be
a note in the nonexclusive-mode backup docs to say so?

Regards,
-Chap