pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_dump, gzwrite, and errno
4+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2020-06-19 bbbce94dc2 e74559c976 5b52008a64 2e155d90d1 ae3259c550 83762d0a92 411febd53e Ensure write failure reports no-disk-space
* pg_dump, gzwrite, and errno
@ 2020-06-11 15:37 Justin Pryzby <pryzby@telsasoft.com>
2020-06-18 21:20 ` Re: pg_dump, gzwrite, and errno Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 4+ messages in thread
From: Justin Pryzby @ 2020-06-11 15:37 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
While testing Pavel's patch for pg_dump --filter, I got:
pg_dump: error: could not write to output file: Success
[pryzbyj@database postgresql]$ echo $?
1
I see we tried to fix it few years ago:
https://www.postgresql.org/message-id/flat/1498120508308.9826%40infotecs.ru
https://www.postgresql.org/message-id/flat/20160125143008.2539.2878%40wrigleys.postgresql.org
https://www.postgresql.org/message-id/20160307.174354.251049100.horiguchi.kyotaro@lab.ntt.co.jp
https://www.postgresql.org/message-id/20150608174336.GM133018@postgresql.org
Commits:
4d57e83816778c6f61ea35c697f937a6f9c3c3de
9a3b5d3ad0f1c19c47e2ee65b372344cb0616c9a
This patch fixes it for me
pg_dump: error: could not write to output file: No space left on device
inline patch 1: src/bin/pg_dump/pg_backup_directory.c (+4/-0 lines) download inline-1.patch
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -347,8 +347,12 @@ _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
lclContext *ctx = (lclContext *) AH->formatData;
if (dLen > 0 && cfwrite(data, dLen, ctx->dataFH) != dLen)
+ {
+ if (errno == 0)
+ errno = ENOSPC;
fatal("could not write to output file: %s",
get_cfp_error(ctx->dataFH));
+ }
}
PS. Due to $UserError, I originally sent this message with inaccurate RFC822
headers..
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_dump, gzwrite, and errno
2020-06-11 15:37 pg_dump, gzwrite, and errno Justin Pryzby <pryzby@telsasoft.com>
@ 2020-06-18 21:20 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-06-18 21:30 ` Re: pg_dump, gzwrite, and errno Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Alvaro Herrera @ 2020-06-18 21:20 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: pgsql-hackers@postgresql.org, Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
On 2020-Jun-11, Justin Pryzby wrote:
> --- a/src/bin/pg_dump/pg_backup_directory.c
> +++ b/src/bin/pg_dump/pg_backup_directory.c
> @@ -347,8 +347,12 @@ _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
> lclContext *ctx = (lclContext *) AH->formatData;
>
> if (dLen > 0 && cfwrite(data, dLen, ctx->dataFH) != dLen)
> + {
> + if (errno == 0)
> + errno = ENOSPC;
> fatal("could not write to output file: %s",
> get_cfp_error(ctx->dataFH));
> + }
> }
This seems correct to me. (I spent a long time looking at zlib sources
to convince myself that it does work with compressed files too). There
are more calls to cfwrite in pg_backup_directory.c though -- we should
patch them all.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_dump, gzwrite, and errno
2020-06-11 15:37 pg_dump, gzwrite, and errno Justin Pryzby <pryzby@telsasoft.com>
2020-06-18 21:20 ` Re: pg_dump, gzwrite, and errno Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-06-18 21:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-19 21:30 ` Re: pg_dump, gzwrite, and errno Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2020-06-18 21:30 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@postgresql.org, Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Jun-11, Justin Pryzby wrote:
>> --- a/src/bin/pg_dump/pg_backup_directory.c
>> +++ b/src/bin/pg_dump/pg_backup_directory.c
>> @@ -347,8 +347,12 @@ _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
>> lclContext *ctx = (lclContext *) AH->formatData;
>>
>> if (dLen > 0 && cfwrite(data, dLen, ctx->dataFH) != dLen)
>> + {
>> + if (errno == 0)
>> + errno = ENOSPC;
>> fatal("could not write to output file: %s",
>> get_cfp_error(ctx->dataFH));
>> + }
>> }
> This seems correct to me.
Surely it's insufficient as-is, because there is no reason to suppose
that errno is zero at entry. You'd need to set errno = 0 first.
Also it's fairly customary in our sources to include a comment about
this machination; so the full ritual is usually more like
errno = 0;
if (pg_pwrite(fd, data, len, xlrec->offset) != len)
{
/* if write didn't set errno, assume problem is no disk space */
if (errno == 0)
errno = ENOSPC;
ereport ...
> (I spent a long time looking at zlib sources
> to convince myself that it does work with compressed files too)
Yeah, it's not obvious that gzwrite has the same behavior w.r.t. errno
as a plain write. But there's not much we can do to improve matters
if it does not, so we might as well assume it does.
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_dump, gzwrite, and errno
2020-06-11 15:37 pg_dump, gzwrite, and errno Justin Pryzby <pryzby@telsasoft.com>
2020-06-18 21:20 ` Re: pg_dump, gzwrite, and errno Alvaro Herrera <alvherre@2ndquadrant.com>
2020-06-18 21:30 ` Re: pg_dump, gzwrite, and errno Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-06-19 21:30 ` Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 4+ messages in thread
From: Alvaro Herrera @ 2020-06-19 21:30 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@postgresql.org, Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
On 2020-Jun-18, Tom Lane wrote:
> Surely it's insufficient as-is, because there is no reason to suppose
> that errno is zero at entry. You'd need to set errno = 0 first.
Oh, right.
> Also it's fairly customary in our sources to include a comment about
> this machination; so the full ritual is usually more like
Yeah, I had that in my local copy. Done like that in all the most
obvious places. But there are more places that are still wrong: I
believe every single place that calls WRITE_ERROR_EXIT is doing the
wrong thing.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2020-06-19 21:30 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 15:37 pg_dump, gzwrite, and errno Justin Pryzby <pryzby@telsasoft.com>
2020-06-18 21:20 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-06-18 21:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-19 21:30 ` Alvaro Herrera <alvherre@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox