Re: increasing the default WAL segment size

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Beena Emerson <memissemerson@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-09-06T02:07:07Z
Lists: pgsql-hackers

Attachments

Hi,

I was looking to commit this, but the changes I made ended up being
pretty large. Here's what I changed in the attached:
- split GUC_UNIT_BYTE into a separate commit, squashed rest
- renamed GUC_UNIT_BYT to GUC_UNIT_BYTE, don't see why we'd have such a
  weird abbreviation?
- bumped control file version, otherwise things wouldn't work correctly
- wal_segment_size text still said "Shows the number of pages per write
  ahead log segment."
- I still feel strongly that exporting XLogSegSize, which previously was
  a macro and now a integer variable, is a bad idea. Hence I've renamed
  it to wal_segment_size.
- There still were comments referencing XLOG_SEG_SIZE
- IsPowerOf2 regarded 0 as a valid power of two
- ConvertToXSegs() depended on a variable not passed as arg, bad idea.
- As previously mentioned, I don't think it's ok to rely on vars like
  XLogSegSize to be defined both in backend and frontend code.
- I don't think XLogReader can rely on XLogSegSize, needs to be
  parametrized.
- pg_rewind exported another copy of extern int XLogSegSize
- streamutil.h had a extern uint32 WalSegsz; but used
  RetrieveXlogSegSize, that seems needlessly different
- moved wal_segment_size (aka XLogSegSize) to xlog.h
- pg_standby included xlogreader, not sure why?
- MaxSegmentsPerLogFile still had a conflicting naming scheme
- you'd included "sys/stat.h", that's not really appropriate for system
  headers, should be <sys/stat.h> (and then grouped w/ rest)
- pg_controldata's warning about an invalid segsize missed newlines

Unresolved:
- this needs some new performance tests, the number of added instructions
  isn't trivial. Don't think there's anything, but ...
- read through it again, check long lines
- pg_standby's RetrieveWALSegSize() does too much for it's name. It
  seems quite weird that a function named that way has the section below
  "/* check if clean up is necessary */"
- the way you redid the ReadControlFile() invocation doesn't quite seem
  right. Consider what happens if XLOGbuffers isn't -1 - then we
  wouldn't read the control file, but you unconditionally copy it in
  XLOGShmemInit(). I think we instead should introduce something like
  XLOGPreShmemInit() that reads the control file unless in bootstrap
  mode. Then get rid of the second ReadControlFile() already present.
- In pg_resetwal.c:ReadControlFile() we ignore the file contents if
  there's an invalid segment size, but accept the contents as guessed if
  there's a crc failure - that seems a bit weird?
- verify EXEC_BACKEND does the right thing
- not this commit/patch, but XLogReadDetermineTimeline() could really
  use some simplifying of repetitive expresssions
- XLOGShmemInit shouldn't memcpy to temp_cfile and such, why not just
  save previous pointer in a local variable?
- could you fill in the Reviewed-By: line in the commit message?

Running out of concentration / time now.

- Andres

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.