Re: increasing the default WAL segment size

Kuntal Ghosh <kuntalghosh.2007@gmail.com>

From: Kuntal Ghosh <kuntalghosh.2007@gmail.com>
To: Beena Emerson <memissemerson@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-02-24T12:30:45Z
Lists: pgsql-hackers
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

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Make WAL segment size configurable at initdb time.

  2. Perform only one ReadControlFile() during startup.

  3. Introduce BYTES unit for GUCs.

  4. Remove useless duplicate inclusions of system header files.

  5. Refactor other replication commands to use DestRemoteSimple.

  6. Add a SHOW command to the replication command language.

  7. Add a new DestReceiver for printing tuples without catalog access.

  8. Support fls().

  9. Extend yesterday's patch making BLCKSZ and RELSEG_SIZE configurable to also

  10. Commit the reasonably uncontroversial parts of J.R. Nield's PITR patch, to

  11. XLOG (also known as WAL -:)) Bootstrap/Startup/Shutdown.

  12. Transaction log manager core code.