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 <memissemerson@gmail.com> wrote:
>
>
> On Tue, Feb 28, 2017 at 9:45 AM, Jim Nasby <Jim.Nasby@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
