Re: WAL replay bugs

On Tue, Jul 1, 2014 at 7:25 PM, Kyotaro HORIGUCHI <
horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:

> Hello, I had a look on this patch.

Thanks for your comments. Looking forward to seeing some more input.

> - contrib/buffer_capture_cmp/README
>
> - 'contains' seems duplicate in the first paragraph.
> - The second paragraph says that 'This script can use the node
> number of the master node available as the first argument of
> the script when it is run within the test suite.' But test.sh
> seems not giving such a parameter.
>
Yeah right... This was a rest of some previous hacking on this feature.
Paragraph was rather unclear so I rewrote it, mentioning that the custom
script can use PGPORT to connect to the node where tests can be run.

- contrib/buffer_capture_cmp/Makefile
>
> "make check" does nothing when BUFFER_CAPTURE is not defined, as
> described in itself. But I trapped by that after build the
> server by 'make CFLAGS="-DBUFFER_CAPTURE"':( It would be better
> that 'make check' without defining it prints some message.
>
Sure, I added such a message in the makefile.

- buffer_capture_cmp.c
>
> This source generates void executable when BUFFER_CAPTURE is
> not defined. The restriction seems to be put only to use
> BUFFER_CAPTURE_FILE in bufcapt.h. If so, changing the parameter
> of the executable as described in the following comment for
> main() would blow off the necessity for the restriction.
>
Done. The compilation of this utility is now independent on BUFFER_CAPTURE.
At the same time I made test.sh a bit smarter to have it grab the value of
BUFFER_CAPTURE_FILE directly from bufcapt.h.

- buffer_capture_cmp.c/main()
>
> The parameters for this command are the parent directories for
> each capture file. This is a bit inconvenient for separate
> use. For example, when I want to gather the capture files from
> multiple servers then compare them, I should unwillingly make
> their own directories for each capture file. If no particular
> reason exists for the design, I suppose it would be more
> convenient that the parameters are the names of the capture
> files themselves.
>
Fixed. I changed back the utility to directly file names instead of data
folders as arguments.

Updated patches addressing those comments are attached.
Regards,
--
Michael