Re: pg_rewind in contrib

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Satoshi Nagayasu <snaga(at)uptime(dot)jp>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Michael Paquier <mpaquier(at)vmware(dot)com>
Subject: Re: pg_rewind in contrib
Date: 2015-01-06 07:13:13
Message-ID: CAB7nPqQce=h=qE64fVit0pBZte-V8QB8Sg77amzApyM6V+MDQg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 6, 2015 at 2:38 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Here's an updated version of pg_rewind. The code itself is the same as
> before, and corresponds to the sources in the current pg_rewind github
> repository, as of commit a65e3754faf9ca9718e6b350abc736de586433b7. Based
> mostly on Michael's comments, I have:
>
> * replaced VMware copyright notices with PGDG ones.
> * removed unnecessary cruft from .gitignore
> * changed the --version line and "report bugs" notice in --help to match
> other binaries in the PostgreSQL distribution
> * moved documentation from README to the user manual.
> * minor fixes to how the regression tests are launched so that they work
> again
>
> Some more work remains to be done on the regression tests. The way they're
> launched now is quite weird. It was written that way to make it work outside
> the PostgreSQL source tree, and also on Windows. Now that it lives in
> contrib, it should be redesigned.
>
> The documentation could also use some work; for now I just converted the
> existing text from README to sgml format.

Some more comments:
- The binary pg_rewind needs to be ignored in contrib/pg_rewind/
- Be careful that ./launcher permission should be set to 755 when
doing a git add in it... Or regression tests will fail.
- It would be good to get make check working so as it includes both
check-local and check-remote
- installcheck should be a target ignored.
- Nitpicking: the header formats of filemap.c, datapagemap.c,
datapagemap.h and util.h are incorrect (I pushed a fix about that in
pg_rewind itself, feel free to pick it up).
- parsexlog.c has a copyright mention to Nippon Telegraph and
Telephone Corporation. Cannot we drop it safely?
- MSVC build is not supported yet. You need to do something similar to
pg_xlogdump, aka some magic with for example xlogreader.c.
- Error codes needs to be generated before building pg_rewind. If I do
for example a simple configure followed by make I get a failure:
$ ./configure
$ cd contrib/pg_rewind && make
In file included from parsexlog.c:16:
In file included from ../../src/include/postgres.h:48:
../../src/include/utils/elog.h:69:10: fatal error: 'utils/errcodes.h'
file not found
#include "utils/errcodes.h"
- Build fails with MinGW as there is visibly some unportable code:
copy_fetch.c: In function 'recurse_dir':
copy_fetch.c:112:3: warning: implicit declaration of function 'S_ISLNK' [-Wimpli
cit-function-declaration]
else if (S_ISLNK(fst.st_mode))
^
copy_fetch.c: In function 'check_samefile':
copy_fetch.c:298:2: warning: passing argument 2 of '_fstat64i32' from incompatib
le pointer type [enabled by default]
if (fstat(fd1, &statbuf1) < 0)
^
In file included from ../../src/include/port.h:283:0,
from ../../src/include/c.h:1050,
from ../../src/include/postgres_fe.h:25,
from copy_fetch.c:10:
c:\mingw\include\sys\stat.h:200:32: note: expected 'struct _stat64i32 *' but arg
ument is of type 'struct stat *'
__CRT_MAYBE_INLINE int __cdecl _fstat64i32(int desc, struct _stat64i32 *_stat)
{
^
copy_fetch.c:304:2: warning: passing argument 2 of '_fstat64i32' from incompatib
le pointer type [enabled by default]
if (fstat(fd2, &statbuf2) < 0)
^
In file included from ../../src/include/port.h:283:0,
from ../../src/include/c.h:1050,
from ../../src/include/postgres_fe.h:25,
from copy_fetch.c:10:
c:\mingw\include\sys\stat.h:200:32: note: expected 'struct _stat64i32 *' but arg
ument is of type 'struct stat *'
__CRT_MAYBE_INLINE int __cdecl _fstat64i32(int desc, struct _stat64i32 *_stat)
{
^
copy_fetch.c: In function 'truncate_target_file':
copy_fetch.c:436:2: warning: implicit declaration of function 'truncate' [-Wimpl
icit-function-declaration]
if (truncate(dstpath, newsize) != 0)
^
copy_fetch.c: In function 'slurpFile':
copy_fetch.c:561:2: warning: passing argument 2 of '_fstat64i32' from incompatib
le pointer type [enabled by default]
if (fstat(fd, &statbuf) < 0)
^
In file included from ../../src/include/port.h:283:0,
from ../../src/include/c.h:1050,
from ../../src/include/postgres_fe.h:25,
from copy_fetch.c:10:
c:\mingw\include\sys\stat.h:200:32: note: expected 'struct _stat64i32 *' but arg
ument is of type 'struct stat *'
__CRT_MAYBE_INLINE int __cdecl _fstat64i32(int desc, struct _stat64i32 *_stat)
{
^
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -We
ndif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -f
wrapv -fexcess-precision=standard -g -DG -fno-omit-frame-pointer -I../../src/int
erfaces/libpq -I. -I. -I../../src/include -DFRONTEND "-I../../src/include/port/
win32" -c -o libpq_fetch.o libpq_fetch.c -MMD -MP -MF .deps/libpq_fetch.Po
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -We
ndif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -f
wrapv -fexcess-precision=standard -g -DG -fno-omit-frame-pointer -I../../src/int
erfaces/libpq -I. -I. -I../../src/include -DFRONTEND "-I../../src/include/port/
win32" -c -o filemap.o filemap.c -MMD -MP -MF .deps/filemap.Po
filemap.c:12:19: fatal error: regex.h: No such file or directory
#include <regex.h>
^
compilation terminated.
../../src/Makefile.global:732: recipe for target 'filemap.o' failed
make: *** [filemap.o] Error 1
Hm. I think that this is something we should try to fix first
upstream. I feel as well that regression tests are going to need some
tuning as well to work properly.

Regards,
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2015-01-06 07:13:57 Re: Patch to add functionality to specify ORDER BY in CREATE FUNCTION for SRFs
Previous Message Peter Geoghegan 2015-01-06 07:12:11 Re: Problems with approach #2 to value locking (INSERT ... ON CONFLICT UPDATE/IGNORE patch)