Re: pg_basebackup for streaming base backups

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_basebackup for streaming base backups
Date: 2011-01-18 19:12:22
Message-ID: AANLkTi=c=hTSBUeVjJA_Dx61OWkyruQYrKr5mMr7Le_D@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 18, 2011 at 19:20, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> Excerpts from Magnus Hagander's message of mar ene 18 11:53:55 -0300 2011:
>> On Tue, Jan 18, 2011 at 15:49, Alvaro Herrera
>> <alvherre(at)commandprompt(dot)com> wrote:
>> > Also, in messages of this kind,
>> > +               if (gzsetparams(ztarfile, compresslevel, Z_DEFAULT_STRATEGY) != Z_OK)
>> > +               {
>> > +                   fprintf(stderr, _("%s: could not set compression level %i\n"),
>> > +                           progname, compresslevel);
>> >
>> > Shouldn't you also be emitting the gzerror()? ... oh I see you're
>> > already doing it for most gz calls.
>>
>> It's not clear from the zlib documentation I have that gzerror() works
>> after a gzsetparams(). Do you have any docs that say differently by
>> any chance?
>
> Ah, no.  I was reading zlib.h, which is ambiguous as you say:
>
> ZEXTERN int ZEXPORT gzsetparams OF((gzFile file, int level, int strategy));
> /*
>     Dynamically update the compression level or strategy. See the description
>   of deflateInit2 for the meaning of these parameters.
>     gzsetparams returns Z_OK if success, or Z_STREAM_ERROR if the file was not
>   opened for writing.
> */
>
> ZEXTERN const char * ZEXPORT gzerror OF((gzFile file, int *errnum));
> /*
>     Returns the error message for the last error which occurred on the
>   given compressed file. errnum is set to zlib error number. If an
>   error occurred in the file system and not in the compression library,
>   errnum is set to Z_ERRNO and the application may consult errno
>   to get the exact error code.
>
>
> ... but a quick look at the code says that it sets gz_stream->z_err
> which is what gzerror returns:
>
> int ZEXPORT gzsetparams (file, level, strategy)
>    gzFile file;
>    int level;
>    int strategy;
> {
>    gz_stream *s = (gz_stream*)file;
>
>    if (s == NULL || s->mode != 'w') return Z_STREAM_ERROR;
>
>    /* Make room to allow flushing */
>    if (s->stream.avail_out == 0) {
>
>        s->stream.next_out = s->outbuf;
>        if (fwrite(s->outbuf, 1, Z_BUFSIZE, s->file) != Z_BUFSIZE) {
>            s->z_err = Z_ERRNO;
>        }
>        s->stream.avail_out = Z_BUFSIZE;
>    }
>
>    return deflateParams (&(s->stream), level, strategy);
> }

Ah, ok. I've added the errorcode now, PFA. I also fixed an error in
the change for result codes I broke in the last patch. github branch
updated as usual.

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

Attachment Content-Type Size
pg_basebackup.patch text/x-patch 45.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2011-01-18 19:12:53 Re: Spread checkpoint sync
Previous Message Magnus Hagander 2011-01-18 19:04:05 pgsql: Log replication connections only when log_connections is on