On Fri, Feb 24, 2017 at 12:47 PM, Beena Emerson <memissemer...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers