Обсуждение: pg_dump, gzwrite, and errno

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

--- 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..

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

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

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