Re: Confusing error message with too-large file in pg_basebackup

Version: 9.4.5
Summary: confusing error message for too-large file failure in pg_basebackup

Details:

1. PostgreSQL previously core dumped on this system and left behind a
9gb core file, which was never deleted.

2. Attempted to pg_basebackup the server.

3. Got this error message:

pg_basebackup: could not get transaction log end position from server:
ERROR: archive member "core" too large for tar format

This was very confusing to the user, because they weren't requesting tar
format, and even setting -Fp got the same error message. I can only
hypothesize that tar is used somewhere under the hood.

pg_basebackup doesn't need to work under these circumstances, but maybe
we could give a less baffling error message?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Nov 20, 2015 at 9:37 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> pg_basebackup: could not get transaction log end position from server:
> ERROR: archive member "core" too large for tar format

That's a backend-side error.

> This was very confusing to the user, because they weren't requesting tar
> format, and even setting -Fp got the same error message. I can only
> hypothesize that tar is used somewhere under the hood.

Exactly, when a base backup is taken through the replication protocol,
backend always sends it in tar format for performance reasons. It is
then up to pg_basebackup to decide if the output should be untared or
not.

> pg_basebackup doesn't need to work under these circumstances, but maybe
> we could give a less baffling error message?

We would need to let the backend know about the output format expected
by the caller of BASE_BACKUP by extending the command in the
replication protocol. It does not sound like a good idea to me just to
make some potential error messages more verbose.
--
Michael

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Fri, Nov 20, 2015 at 9:37 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>> pg_basebackup: could not get transaction log end position from server:
>> ERROR: archive member "core" too large for tar format

> That's a backend-side error.

>> This was very confusing to the user, because they weren't requesting tar
>> format, and even setting -Fp got the same error message. I can only
>> hypothesize that tar is used somewhere under the hood.

> Exactly, when a base backup is taken through the replication protocol,
> backend always sends it in tar format for performance reasons. It is
> then up to pg_basebackup to decide if the output should be untared or
> not.

It's not unreasonable for pg_basebackup to use tar format, because the
size limitation should not be an issue for files that are expected to
be in a data directory. Leftover core dump files are unexpected :-(.
I wonder if we could put some sort of filter into pg_basebackup so
it would skip this sort of thing.

regards, tom lane

On Fri, Nov 20, 2015 at 1:41 PM, Tom Lane wrote:
> It's not unreasonable for pg_basebackup to use tar format, because the
> size limitation should not be an issue for files that are expected to
> be in a data directory. Leftover core dump files are unexpected :-(.
> I wonder if we could put some sort of filter into pg_basebackup so
> it would skip this sort of thing.

We could try to have some filtering with the core file name for most
of the main distribution cases, like "core", or "core*", however with
kernel.core_pattern it is easy to set up on a given system a custom
core file name format.

Without having to call "file" through system(), another way would be
to have directly a look at the file type, but this looks
unmaintainable to me, look for example here in magic/Magdir/ that
keeps a reference of that. That's quite interesting.
ftp://ftp.astron.com/pub/file/
Now there is actually the possibility to call directly "file" in the
base backup code path as well, and filter the result depending on if
"core" shows up...
--
Michael

Le 20 nov. 2015 6:26 AM, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com> a
écrit :
>
> On Fri, Nov 20, 2015 at 1:41 PM, Tom Lane wrote:
> > It's not unreasonable for pg_basebackup to use tar format, because the
> > size limitation should not be an issue for files that are expected to
> > be in a data directory. Leftover core dump files are unexpected :-(.
> > I wonder if we could put some sort of filter into pg_basebackup so
> > it would skip this sort of thing.
>
> We could try to have some filtering with the core file name for most
> of the main distribution cases, like "core", or "core*", however with
> kernel.core_pattern it is easy to set up on a given system a custom
> core file name format.
>
> Without having to call "file" through system(), another way would be
> to have directly a look at the file type, but this looks
> unmaintainable to me, look for example here in magic/Magdir/ that
> keeps a reference of that. That's quite interesting.
> ftp://ftp.astron.com/pub/file/
> Now there is actually the possibility to call directly "file" in the
> base backup code path as well, and filter the result depending on if
> "core" shows up...

Looking at the file's size is probably a better idea. As far as I know,
PostgreSQL doesn't create files bigger than 1GB, except for log files. I'm
not sure about this but I guess pg_basebackup doesn't ship log files. So,
looking at the size would work.

On Fri, Nov 20, 2015 at 4:39 PM, Guillaume Lelarge wrote:
> Looking at the file's size is probably a better idea.

But isn't that already what the backend does?

> As far as I know,
> PostgreSQL doesn't create files bigger than 1GB, except for log files.

In most cases where the default is used, yes. Now this depends as well
on --with-segsize.

> I'm not sure about this but I guess pg_basebackup doesn't ship log files. So, looking at the size would work.

It does fetch files from pg_log. We actually had on hackers not so
long ago discussions about authorizing some filtering option in
pg_basebackup for partially this purpose.
--
Michael

Guillaume Lelarge wrote:

> Looking at the file's size is probably a better idea. As far as I know,
> PostgreSQL doesn't create files bigger than 1GB, except for log files. I'm
> not sure about this but I guess pg_basebackup doesn't ship log files. So,
> looking at the size would work.

Hmm, so we let configure --with-segsize to change the file size. The
configure help says that the limit should be "less than your OS' limit
on file size". We don't warn them that this could cause backup
problems later on. Should we add a blurb about that somewhere?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Le 20 nov. 2015 1:34 PM, "Alvaro Herrera" <alvherre(at)2ndquadrant(dot)com> a
écrit :
>
> Guillaume Lelarge wrote:
>
> > Looking at the file's size is probably a better idea. As far as I know,
> > PostgreSQL doesn't create files bigger than 1GB, except for log files.
I'm
> > not sure about this but I guess pg_basebackup doesn't ship log files.
So,
> > looking at the size would work.
>
> Hmm, so we let configure --with-segsize to change the file size. The
> configure help says that the limit should be "less than your OS' limit
> on file size". We don't warn them that this could cause backup
> problems later on. Should we add a blurb about that somewhere?
>

If we do, we should already have done so because of the file size limit in
the tar format backup done with pg_dump.

Guillaume Lelarge wrote:
> Le 20 nov. 2015 1:34 PM, "Alvaro Herrera" <alvherre(at)2ndquadrant(dot)com> a
> écrit :

> > Hmm, so we let configure --with-segsize to change the file size. The
> > configure help says that the limit should be "less than your OS' limit
> > on file size". We don't warn them that this could cause backup
> > problems later on. Should we add a blurb about that somewhere?
>
> If we do, we should already have done so because of the file size limit in
> the tar format backup done with pg_dump.

Agreed, please do. I cannot lend you my time machine right now, though,
because I'm using it to go to a past conference I missed, so please ask
someone else.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Guillaume Lelarge wrote:
>> Le 20 nov. 2015 1:34 PM, "Alvaro Herrera" <alvherre(at)2ndquadrant(dot)com> a
>> crit :
>>> Hmm, so we let configure --with-segsize to change the file size. The
>>> configure help says that the limit should be "less than your OS' limit
>>> on file size". We don't warn them that this could cause backup
>>> problems later on. Should we add a blurb about that somewhere?

>> If we do, we should already have done so because of the file size limit in
>> the tar format backup done with pg_dump.

> Agreed, please do. I cannot lend you my time machine right now, though,
> because I'm using it to go to a past conference I missed, so please ask
> someone else.

Um ... the segment size has no effect on pg_dump. It is true that you
can't use pg_dump's -Ft format if the text form of a table's data exceeds
8GB or whatever it is, but it matters not whether the table is segmented
internally. I'm not sure if this limitation is well-documented either,
but it's a completely different issue so far as users are concerned.

regards, tom lane

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Guillaume Lelarge wrote:
>> Looking at the file's size is probably a better idea. As far as I know,
>> PostgreSQL doesn't create files bigger than 1GB, except for log files. I'm
>> not sure about this but I guess pg_basebackup doesn't ship log files. So,
>> looking at the size would work.

> Hmm, so we let configure --with-segsize to change the file size. The
> configure help says that the limit should be "less than your OS' limit
> on file size". We don't warn them that this could cause backup
> problems later on. Should we add a blurb about that somewhere?

Actually ... why don't we get rid of the limit? wikipedia's entry on
tar format says

... only 11 octal digits can be stored. This gives a maximum file size
of 8 gigabytes on archived files. To overcome this limitation, star in
2001 introduced a base-256 coding that is indicated by setting the
high-order bit of the leftmost byte of a numeric field. GNU-tar and
BSD-tar followed this idea.

If that extension is as widespread as this suggests, then following it
when we have a file > 8GB seems like a better answer than failing
entirely. If you try to read the dump with an old tar program, old
pg_restore, etc, it might fail ... but are you really worse off than
if you couldn't make the dump at all?

regards, tom lane

On Fri, 20 Nov 2015 15:20:12 -0500
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Actually ... why don't we get rid of the limit? wikipedia's entry on
> tar format says
>
> ... only 11 octal digits can be stored. This gives a maximum file size
> of 8 gigabytes on archived files. To overcome this limitation, star in
> 2001 introduced a base-256 coding that is indicated by setting the
> high-order bit of the leftmost byte of a numeric field. GNU-tar and
> BSD-tar followed this idea.
>
> If that extension is as widespread as this suggests, then following it
> when we have a file > 8GB seems like a better answer than failing
> entirely. If you try to read the dump with an old tar program, old
> pg_restore, etc, it might fail ... but are you really worse off than
> if you couldn't make the dump at all?
>
> regards, tom lane

+1

-dg

--
David Gould daveg(at)sonic(dot)net
If simplicity worked, the world would be overrun with insects.

On 11/20/2015 12:20 PM, Tom Lane wrote:
> Actually ... why don't we get rid of the limit? wikipedia's entry on
> tar format says

We still shouldn't be dumping core files as part of a pg_basebackup...
but then, why was the core file even IN the $PGDATA directory, shouldn't
it have been in some other dir, like the postgres daemon user's $HOME ?

--
john r pierce, recycling bits in santa cruz

John R Pierce <pierce(at)hogranch(dot)com> writes:
> We still shouldn't be dumping core files as part of a pg_basebackup...

It'd be reasonable to skip 'em if we can identify 'em reliably. I'm
not sure how reliably we can do that though.

> but then, why was the core file even IN the $PGDATA directory, shouldn't
> it have been in some other dir, like the postgres daemon user's $HOME ?

No, that's standard behavior. (I know of no Unix-oid system that dumps
cores into your $HOME by default; it's normally either $PWD or some
reserved directory like /cores.)

regards, tom lane

On 11/20/2015 2:13 PM, Tom Lane wrote:
> It'd be reasonable to skip 'em if we can identify 'em reliably. I'm
> not sure how reliably we can do that though.

aren't they nearly always named 'core' ?

--
john r pierce, recycling bits in santa cruz

John R Pierce <pierce(at)hogranch(dot)com> writes:
> On 11/20/2015 2:13 PM, Tom Lane wrote:
>> It'd be reasonable to skip 'em if we can identify 'em reliably. I'm
>> not sure how reliably we can do that though.

> aren't they nearly always named 'core' ?

No. Modern systems more often call them something like 'core.<pid>'.
What really makes it messy is that the name is user-configurable on
most Linux kernels, see /proc/sys/kernel/core_pattern.

We could probably get away with excluding anything that matches "*core*",
but it wouldn't be bulletproof.

regards, tom lane

I wrote:
> Actually ... why don't we get rid of the limit? wikipedia's entry on
> tar format says
> ... only 11 octal digits can be stored. This gives a maximum file size
> of 8 gigabytes on archived files. To overcome this limitation, star in
> 2001 introduced a base-256 coding that is indicated by setting the
> high-order bit of the leftmost byte of a numeric field. GNU-tar and
> BSD-tar followed this idea.
> If that extension is as widespread as this suggests, then following it
> when we have a file > 8GB seems like a better answer than failing
> entirely. If you try to read the dump with an old tar program, old
> pg_restore, etc, it might fail ... but are you really worse off than
> if you couldn't make the dump at all?

I looked into the GNU tar sources and confirmed that gtar supports this
concept. (It looks from the GNU sources like they've supported it for
a *really long time*, like since the 90's, in which case wikipedia's
credit to "star" for the idea is full of it.)

Hence, I propose something like the attached (WIP, has no doc
corrections). I've done simple testing on the pg_dump/pg_restore code
path, but not on basebackup --- anyone want to test that?

I'm not sure whether we should treat this as a back-patchable bug fix
or a new feature for HEAD only. If we don't back-patch it, there are
in any case several bugs here that we must fix. In particular, the
existing coding in ReceiveTarFile:

size_t filesz = 0;
...
sscanf(&tarhdr[124], "%11o", (unsigned int *) &filesz);

is utterly, absolutely, completely broken; it'll fail grossly on
any 64-bit big-endian hardware. There are other places with misplaced
faith that "unsigned long" is at least as wide as size_t.

Comments?

regards, tom lane

On Fri, 20 Nov 2015 19:11:23 -0500
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I'm not sure whether we should treat this as a back-patchable bug fix
> or a new feature for HEAD only. If we don't back-patch it, there are
> in any case several bugs here that we must fix. In particular, the
> existing coding in ReceiveTarFile:
>
> size_t filesz = 0;
> ...
> sscanf(&tarhdr[124], "%11o", (unsigned int *) &filesz);
>
> is utterly, absolutely, completely broken; it'll fail grossly on
> any 64-bit big-endian hardware. There are other places with misplaced
> faith that "unsigned long" is at least as wide as size_t.
>
> Comments?

My vote would be that it should go in 9.5. If it gets back patched then
some dumps produced by 9.4.x would not be readable by 9.4.x-1. But no 9.5.x
dump is broken by changing it now.

-dg

--
David Gould 510 282 0869 daveg(at)sonic(dot)net
If simplicity worked, the world would be overrun with insects.

David Gould <daveg(at)sonic(dot)net> writes:
> My vote would be that it should go in 9.5. If it gets back patched then
> some dumps produced by 9.4.x would not be readable by 9.4.x-1. But no 9.5.x
> dump is broken by changing it now.

The thing is, though, that without the patch 9.4.x would have failed to
produce such a dump at all. Is that really a better outcome? At least
if you have the dump, you have the option to update so you can read it.
With no dump at all, you might be screwed at a most inopportune moment.
(Think "automatic dump script was failing and nobody noticed" ...)

regards, tom lane

On Sat, Nov 21, 2015 at 7:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> John R Pierce <pierce(at)hogranch(dot)com> writes:
>> On 11/20/2015 2:13 PM, Tom Lane wrote:
>>> It'd be reasonable to skip 'em if we can identify 'em reliably. I'm
>>> not sure how reliably we can do that though.
>
>> aren't they nearly always named 'core' ?
>
> No. Modern systems more often call them something like 'core.<pid>'.
> What really makes it messy is that the name is user-configurable on
> most Linux kernels, see /proc/sys/kernel/core_pattern.
>
> We could probably get away with excluding anything that matches "*core*",
> but it wouldn't be bulletproof.

It does not look like a good idea to me. I have no doubts that there
are deployments including configuration files with such abbreviations
in PGDATA.
--
Michael

On Sat, 21 Nov 2015 14:16:56 +0900
Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:

> On Sat, Nov 21, 2015 at 7:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > John R Pierce <pierce(at)hogranch(dot)com> writes:
> >> On 11/20/2015 2:13 PM, Tom Lane wrote:
> >>> It'd be reasonable to skip 'em if we can identify 'em reliably. I'm
> >>> not sure how reliably we can do that though.
> >
> >> aren't they nearly always named 'core' ?
> >
> > No. Modern systems more often call them something like 'core.<pid>'.
> > What really makes it messy is that the name is user-configurable on
> > most Linux kernels, see /proc/sys/kernel/core_pattern.
> >
> > We could probably get away with excluding anything that matches "*core*",
> > but it wouldn't be bulletproof.
>
> It does not look like a good idea to me. I have no doubts that there
> are deployments including configuration files with such abbreviations
> in PGDATA.

Perhaps matching *core* and size > 100MB or so would cover that.

-dg

--
David Gould 510 282 0869 daveg(at)sonic(dot)net
If simplicity worked, the world would be overrun with insects.

I wrote:
> I looked into the GNU tar sources and confirmed that gtar supports this
> concept. (It looks from the GNU sources like they've supported it for
> a *really long time*, like since the 90's, in which case wikipedia's
> credit to "star" for the idea is full of it.)

I've now also verified that the "tar" present on OS X, which is bsdtar,
handles the GNU base-256 convention just fine, and has done so at least
since Snow Leopard (released 2009).

> Hence, I propose something like the attached (WIP, has no doc
> corrections). I've done simple testing on the pg_dump/pg_restore code
> path, but not on basebackup --- anyone want to test that?

The attached updated patch fixes the pg_dump docs (pg_basebackup doesn't
address the point, so nothing to fix there), and just for cleanliness'
sake, gets rid of all use of sprintf and sscanf to process tar headers.
I've now tested the basebackup path, and in addition done testing on a
32-bit machine, which caused me to realize that the existing code is
even more broken than I thought. Quite aside from the documented 8GB
limitation, we have these issues:

* tarCreateHeader's file-size argument is size_t, which means that on
32-bit machines it will write a corrupt tar header for file sizes between
4GB and 8GB, even though no error is raised (if pgoff_t is 64 bits, which
it will be on just about any remotely modern platform). This is very
very bad: you could have an unreadable backup and not know it. We
evidently broke this in 9.3 while refactoring the tar-writing logic so
basebackup could use it.

* pg_restore fails on tables of size between 4GB and 8GB on machines
where either size_t or unsigned long is 32 bits, even if the archive was
written by a 64-bit machine and hence is not corrupted by the previous
bug. If you use Windows 64 you don't even need to transport the archive
to a different machine to get bitten by this. This bug goes back a very
long way.

* pg_basebackup will fail if there are files of size between 4GB and 8GB,
even on 64-bit machines, because it tells sscanf to read the file size
as "unsigned int".

* As I noted yesterday, pg_basebackup fails entirely, for any file size,
on 64-bit big-endian machines.

These all seem to me to be must-fix issues even if we didn't want to
back-port use of the base-256 convention. Seeing that we have to deal
with these things, I'm inclined to just back-port the whole patch to
all branches.

regards, tom lane

On Sun, Nov 22, 2015 at 5:58 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>wrote:

> These all seem to me to be must-fix issues even if we didn't want to
> back-port use of the base-256 convention. Seeing that we have to deal
> with these things, I'm inclined to just back-port the whole patch to
> all branches.
>

FWIW, patch has been pushed as 00cdd83 and backpatched down to 9.1.
--
Michael