Hi all, > Here's v2, rebased onto the latest master.
I've reviewed this patch. The patch builds against the master (commit e9d4001ec592bcc9a3332547cb1b0211e8794f38) and passes all the tests. The patch does what it intends to do, namely store the kind of the last checkpoint in the control file and display it in the output of the pg_control_checkpoint() function and pg_controldata utility. I did not test it with restartpoints though. Speaking of the torn writes, the size of the control file with this patch applied does not exceed 8Kb. A few code comments: + char ckpt_kind[2 * MAXPGPATH]; I don't completely understand why 2 * MAXPGPATH is used here for the buffer size. [1] defines MAXPGPATH to be standard size of a pathname buffer. How does it relate to ckpt_kind ? + ControlFile->checkPointKind = 0; It is worth a comment that 0 is unknown, as for instance in [2] + (flags == 0) ? "unknown" : "", That reads as if this patch would introduce a new "unknown" checkpoint state. Why have it here at all if after for example initdb the kind is "shutdown" ? + snprintf(ckpt_kind, 2 * MAXPGPATH, "%s%s%s%s%s%s%s%s%s", + (flags == 0) ? "unknown" : "", + (flags & CHECKPOINT_IS_SHUTDOWN) ? "shutdown " : "", + (flags & CHECKPOINT_END_OF_RECOVERY) ? "end-of-recovery " : "", + (flags & CHECKPOINT_IMMEDIATE) ? "immediate " : "", + (flags & CHECKPOINT_FORCE) ? "force " : "", + (flags & CHECKPOINT_WAIT) ? "wait " : "", + (flags & CHECKPOINT_CAUSE_XLOG) ? "wal " : "", + (flags & CHECKPOINT_CAUSE_TIME) ? "time " : "", + (flags & CHECKPOINT_FLUSH_ALL) ? "flush-all" : ""); The space at the strings' end (as in "wait " or "immediate ") introduces extra whitespace in the output of pg_control_checkpoint(). A similar check at [3] places whitespace differently; that arrangement of whitespace should remove the issue. [1] https://github.com/postgres/postgres/blob/410aa248e5a883fde4832999cc9b23c7ace0f2ff/src/include/pg_config_manual.h#L106 [2] https://github.com/postgres/postgres/blob/410aa248e5a883fde4832999cc9b23c7ace0f2ff/src/interfaces/libpq/fe-exec.c#L1078 [3] https://github.com/postgres/postgres/blob/27b77ecf9f4d5be211900eda54d8155ada50d696/src/backend/access/transam/xlog.c#L8851-L8859 Regards, Sergey Dudoladov
