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. -- With Regards, Ashutosh Sharma. EnterpriseDB: http://www.enterprisedb.com On Tue, Feb 28, 2017 at 10:36 AM, Beena Emerson <memissemer...@gmail.com> wrote: > > > On Tue, Feb 28, 2017 at 9:45 AM, Jim Nasby <jim.na...@bluetreble.com> wrote: >> >> On 2/24/17 6:30 AM, Kuntal Ghosh wrote: >>> >>> * You're considering any WAL file with a power of 2 as valid. Suppose, >>> the correct WAL seg size is 64mb. For some reason, the server >>> generated a 16mb invalid WAL file(maybe it crashed while creating the >>> WAL file). Your code seems to treat this as a valid file which I think >>> is incorrect. Do you agree with that? >> >> >> Detecting correct WAL size based on the size of a random WAL file seems >> like a really bad idea to me. >> >> >> I also don't see the reason for #2... or is that how initdb writes out the >> correct control file? > > > The initdb module reads the size from the option provided and sets the > environment variable. This variable is read in > src/backend/access/transam/xlog.c and the ControlFile written. > Unlike pg_resetwal and pg_rewind, pg_basebackup cannot access the Control > file. It only accesses the wal log folder. So we get the XLogSegSize from > the SHOW command using replication connection. > As Kuntal pointed out, I might need to set it from pg_receivewal.c as well. > > Thank you, > > Beena Emerson > > EnterpriseDB: https://www.enterprisedb.com/ > The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
