Re: increasing the default WAL segment size
Kuntal Ghosh <kuntalghosh.2007@gmail.com>
On Fri, Feb 24, 2017 at 12:47 PM, Beena Emerson <memissemerson@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
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Make WAL segment size configurable at initdb time.
- fc49e24fa69a 11.0 landed
-
Perform only one ReadControlFile() during startup.
- 8356753c212a 11.0 landed
-
Introduce BYTES unit for GUCs.
- 6e7baa322773 11.0 landed
-
Remove useless duplicate inclusions of system header files.
- 9e3755ecb2d0 10.0 cited
-
Refactor other replication commands to use DestRemoteSimple.
- bbd8550bce14 10.0 landed
-
Add a SHOW command to the replication command language.
- d1ecd539477f 10.0 landed
-
Add a new DestReceiver for printing tuples without catalog access.
- a84069d93504 10.0 landed
-
Support fls().
- 4f658dc851a7 9.2.0 cited
-
Extend yesterday's patch making BLCKSZ and RELSEG_SIZE configurable to also
- cf9f6c8d8e9d 8.4.0 cited
-
Commit the reasonably uncontroversial parts of J.R. Nield's PITR patch, to
- c3c09be34b6b 8.0.0 cited
-
XLOG (also known as WAL -:)) Bootstrap/Startup/Shutdown.
- 47937403676d 7.1.1 cited
-
Transaction log manager core code.
- 30659d43eb73 7.1.1 cited