Hi David, Andrey, Tested v13 on master (04c4c1c3a96). recovery, pg_rewind, pg_basebackup and regress all pass. Two comments.
1. The pg_resetwal sentence in pg_basebackup.sgml I asked for the field to be documented and still think it should be, but the wording offers pg_resetwal as a way out and it is not one. It clears the flag and resets the WAL, but leaves backup_label, so the cluster still does not start. The flag is gone by then, so the hint v13 just conditioned away is back, and the server itself says "try removing the file backup_label". They do, and it starts silently inconsistent. Unpatched master behaves identically, what is new is that users get sent there. Say what clears the flag, but make restoring backup_label the remedy. Also rename the "pg_resetwal clears the flag" subtest in 057. Separately, pg_resetwal never mentions backup_label in code or docs, and always leaves an unstartable cluster when one is present. A warning there would cut this off at the first step. Happy to post that as its own patch. 2. The assert in backup_control_file() Michael, this one is for you since David left the call to you. Assert builds only, so no production impact, but the invariant is false and I would drop the block rather than narrow it. Only xlog_redo() for XLOG_CHECKPOINT_ONLINE updates the struct without calling UpdateControlFile, so checkPointCopy.nextXid is the only field that can differ, and InitWalRecovery replaces checkPointCopy wholesale when backup_label is present. The copy we ship is correct either way, and asserting it equals the file contradicts the premise that shared memory is authoritative. One detail on Andrey's recipe: a checkpoint replayed before the backup never trips it, since do_pg_backup_start forces a restartpoint. Only replay during file sending matters. Otherwise patch 01 looks ready to me. Thanks, Shihao
