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-09 07:34:52
Message-ID: CAB7nPqQNcb+k7opaCqq6pcPyqbCcrfbt40QzY0qCYmBZHsXgcw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 9, 2015 at 1:02 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Fixed all the errors I got on MSVC. The biggest change was rewriting the
> code that determines if a file is a relation file, based on its filename. It
> used a regular expression, which I replaced with a bunch of sscanf calls,
> and a cross-check that GetRelationPath() returns the same filename.
It looks definitely better like that. Thanks.

>> 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)
>> {
>
>
> Strange. There isn't anything special about the fstat() calls in pg_rewind.
> Do you get these from other modules that call fstat, e.g.
> pg_stat_statements?
>
> I did not see these warnings when building with MSVC, and don't have MinGW
> installed currently.
Don't worry about those ones, it is discussed here already:
http://www.postgresql.org/message-id/CAB7nPqTrmmZo2y92DfZEd-mWo1cenEoaUhCZppv=OB84-4C9vA@mail.gmail.com

MSVC build still has a warning:
"C:\Users\mpaquier\git\postgres\pg_rewind.vcxproj" (default target) (60) ->
(Link target) ->
xlogreader.obj : warning LNK4049: locally defined symbol
pg_crc32c_table imported
[C:\Users\ioltas\git\postgres\pg_rewind.vcxproj]

The documentation needs some more polishing:
1) s/pg_reind/pg_rewind
2) by settings "wal_log_hints = on" => by setting
<varname>wal_log_hints</> to <literal>on</>
3) You should avoid using an hardcoded list of items in a block <para>
to list how pg_rewind works. Each item in the list should be changed
to use <orderedlist>:
<para>
The basic idea is to copy everything from the blah...

<orderedlist>
<listitem>
<para>
Scan the WAL log of blah..
</para>
</listitem>
<listitem>
<para>
paragraph2
</para>
<listitem>
[blah.]
</orderedlist>
</para>
4) --source-server and --target-pgdata are not listed in the list of options.
5) The synopsis should be written like that IMO: pg_rewind [option...]
6) pg_rewind is listed several times, doesn't it need <application>?
7) pg_xlog should use <filename>
8) Perhaps a section "See also" at the bottom could be added to
mention pg_basebackup?

Regards,
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Lukas Fittl 2015-01-09 07:37:41 Re: List of table names of a DB
Previous Message Mark Kirkwood 2015-01-09 06:56:15 Re: List of table names of a DB