Re: pg_basebackup for streaming base backups - Mailing list pgsql-hackers

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@commandprompt.com> wrote:
> > Excerpts from Magnus Hagander's message of mar ene 18 10:47:03 -0300 2011:
> >
> >> Ok, thanks for clarifying. I've updated to use strerror(). Guess it's
> >> time for another patch, PFA :-)
> >
> > Thanks ...  Message nitpick:
> > +   if (compresslevel > 0)
> > +   {
> > +       fprintf(stderr,
> > +               _("%s: this build does not support compression\n"),
> > +               progname);
> > +       exit(1);
> > +   }
> >
> > pg_dump uses the following wording:
> >
> > "WARNING: archive is compressed, but this installation does not support "
> > "compression -- no data will be available\n"
> >
> > So perhaps yours should s/build/installation/
> 
> That shows up when a the *archive* is compressed, though? There are a
> number of other cases that use build in the backend, such as:
> src/backend/utils/misc/guc.c:               errmsg("assertion checking is not
> supported by this build")));
> src/backend/utils/misc/guc.c:                 errmsg("Bonjour is not supported by
> this build")));
> src/backend/utils/misc/guc.c:                 errmsg("SSL is not supported by this
> build")));

Hmm.  I think I'd s/build/installation/ on all those messages for
consistency.

> > 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
theseparameters.    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
errornumber. If an  error occurred in the file system and not in the compression library,  errnum is set to Z_ERRNO and
theapplication 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);
}

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support