Re: increasing the default WAL segment size

On Fri, Feb 24, 2017 at 12:47 PM, Beena Emerson <memissemerson(at)gmail(dot)com> wrote:
>
> Hello,
>
> The recent commit c29aff959dc64f7321062e7f33d8c6ec23db53d has again changed
> the code and the second patch cannot be applied cleanly. Please find
> attached the rebased 02 patch. 01 patch is the same .
>
I've done an initial review of the patch. The objective of the patch
is to modify the wal-segsize as an initdb-time parameter instead of a
compile time parameter.

The patch introduces following three different techniques to expose
the XLogSize to different modules:

1. Directly read XLogSegSize from the control file
This is used by default, i.e., StartupXLOG() and looks good to me.

2. Run the SHOW wal_segment_size command to fetch and set the XLogSegSize

+ if (!RetrieveXLogSegSize(conn))
+ disconnect_and_exit(1);
+
You need the same logic in pg_receivewal.c as well.

3. Retrieve the XLogSegSize by reading the file size of WAL files
+ if (private.inpath != NULL)
+ sprintf(full_path, "%s/%s", private.inpath, fname);
+ else
+ strcpy(full_path, fname);
+
+ stat(full_path, &fst);
+
+ if (!IsValidXLogSegSize(fst.st_size))
+ {
+ fprintf(stderr,
+ _("%s: file size %d is invalid \n"),
+ progname, (int) fst.st_size);
+
+ return EXIT_FAILURE;
+
+ }
+
+ XLogSegSize = (int) fst.st_size;
I see couple of issues with this approach:

* You should check the return value of stat() before going ahead.
Something like,
if (stat(filename, &fst) < 0)
error "file doesn't exist"

* 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?

Is it possible to unify these different techniques of reading
XLogSegSize in a generalized function with a proper documentation
describing the scope and limitations of each approach?

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com