From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: WAL replay bugs |
Date: | 2014-07-02 05:22:35 |
Message-ID: | CAB7nPqSKFdj5oAC3-=+k-MV27MeRpOGUbkFO_7r7eY6Dy64T+A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Move-SEQ_MAGIC-to-sequence.h.patch | text/x-diff | 1.2 KB |
0002-Extract-generic-bash-initialization-process-from-pg_.patch | text/x-diff | 4.6 KB |
0003-Buffer-capture-facility-check-WAL-replay-consistency.patch | text/x-diff | 39.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Abhijit Menon-Sen | 2014-07-02 05:24:05 | Re: 9.5 CF1 |
Previous Message | Craig Ringer | 2014-07-02 05:21:46 | Re: pg_xlogdump --stats |