Hello, On Tue, Mar 7, 2017 at 10:46 AM, Ashutosh Sharma <ashu.coe...@gmail.com> wrote:
> Hi, > > I took a look at this patch. Overall, the patch looks good to me. > However, there are some review comments that I would like to share, > > 1. I think the macro 'PATH_MAX' used in pg_waldump.c file is specific > to Linux. It needs to be changed to some constant value or may be > MAXPGPATH inorder to make it platform independent. > > 2. As already mentioned by Jim and Kuntal upthread, you are trying to > detect the configured WAL segment size in pg_waldump.c and > pg_standby.c files based on the size of the random WAL file which > doesn't look like a good idea. But, then I think the only option we > have is to pass the location of pg_control file to pg_waldump module > along with the start and end wal segments. > > 3. When trying to compile '02-initdb-walsegsize-v2.patch' on Windows, > I got this warning message, > > Warning 1 warning C4005: 'DEFAULT_XLOG_SEG_SIZE' : macro > redefinition > c:\users\ashu\postgresql\src\include\pg_config_manual.h 20 > > Apart from these, I am not having any comments as of now. I am still > validating the patch on Windows. If I find any issues i will update > it. > > Thank you for your reviews Kuntal, Jim, Ashutosh Attached in an updated 02 patch which: 1. Call RetrieveXLogSegSize(conn) in pg_receivewal.c 2. Remove the warning in Windows 3. Change PATH_MAX in pg_waldump with MAXPGPATH Regarding the usage of the wal file size as the XLogSegSize, I agree with what Robert has said. Generally, the wal size will be of the expected wal_segment_size and to have it any other size, esspecially of a valid power2 value is extremely rare and I feel it is not a major cause of concern. Thank you, Beena Emerson EnterpriseDB: http://www.enterprisedb.com
01-add-XLogSegmentOffset-macro.patch
Description: Binary data
02-initdb-walsegsize-v3.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
