Date: Tue, 31 Mar 2020 17:22:29 +0200
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: <pgsql-bugs@lists.postgresql.org>
Cc: Michael Paquier <michael@paquier.xyz>
Subject: [BUG] non archived WAL removed during production crash recovery
Message-ID: <20200331172229.40ee00dc@firost>
Organization: Dalibo
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="MP_/WC6WEc8gEbd3qV4j9CEjgao"
Precedence: bulk

--MP_/WC6WEc8gEbd3qV4j9CEjgao
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hello,

A colleague of mine reported an expected behavior.

On production cluster is in crash recovery, eg. after killing a backend, the
WALs ready to be archived are removed before being archived.

See in attachment the reproduction script "non-arch-wal-on-recovery.bash".

This behavior has been introduced in 78ea8b5daab9237fd42d7a8a836c1c451765499f.
Function XLogArchiveCheckDone() badly consider the in crashed recovery
production cluster as a standby without archive_mode=always. So the check
conclude the WAL can be removed safely.

  bool inRecovery = RecoveryInProgress();
  
  /*
   * The file is always deletable if archive_mode is "off".  On standbys
   * archiving is disabled if archive_mode is "on", and enabled with
   * "always".  On a primary, archiving is enabled if archive_mode is "on"
   * or "always".
   */
  if (!((XLogArchivingActive() && !inRecovery) ||
        (XLogArchivingAlways() && inRecovery)))
      return true;

Please find in attachment a patch that fix this issue using the following test
instead:

  if (!((XLogArchivingActive() && !StandbyModeRequested) ||
        (XLogArchivingAlways() && inRecovery)))
      return true;

I'm not sure if we should rely on StandbyModeRequested for the second part of
the test as well thought. What was the point to rely on RecoveryInProgress() to
get the recovery status from shared mem?

Regards,

--MP_/WC6WEc8gEbd3qV4j9CEjgao
Content-Type: application/octet-stream; name=non-arch-wal-on-recovery.bash
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename=non-arch-wal-on-recovery.bash

IyEvYmluL2Jhc2gKCnNob3B0IC1zIC1vIGVycmV4aXQgbm91bnNldAoKIyBlbnYKUFJPSj1ub24t
YXJjaC13YWwtb24tcmVjb3ZlcnkKUEdQUkVGSVg9L3RtcC8iJFBST0oiClBHQklOPSIkUEdQUkVG
SVgiL2JpbgpQR0RBVEE9IiR7UEdQUkVGSVh9L2RhdGEiClBHTE9HPSIke1BHUFJFRklYfS9wb3N0
Z3Jlc3EubG9nIgoKIyBjbGVhbnVwIHByZXZpb3VzIHRlc3QKWyAtZiAiJHtQR0RBVEF9L3Bvc3Rt
YXN0ZXIucGlkIiBdICYmICR7UEdCSU59L3BnX2N0bCAtcyAtRCAiJFBHREFUQSIgLVcgLW0gaW1t
ZWRpYXRlIHN0b3AKcm0gLXJmICIke1BHREFUQX0iCm1rZGlyIC1wICIke1BHREFUQX0iCgojIGlu
aXQgbmV3IGNsdXN0ZXIKIiRQR0JJTiIvaW5pdGRiIC1VIHBvc3RncmVzIC1OIC1EICIkUEdEQVRB
IiAmPiAvZGV2L251bGwKCiMgc3RhcnQgY2x1c3RlcgpQQVJBTVM9KAogICAgLS13YWxfbGV2ZWw9
cmVwbGljYQogICAgLS1hcmNoaXZlX21vZGU9b24KICAgIC0tYXJjaGl2ZV9jb21tYW5kPQogICAg
LS1sb2dfbWluX21lc3NhZ2VzPWRlYnVnMgogICAgLS1jbHVzdGVyX25hbWU9IiR7UFJPSn0iCikK
IiRQR0JJTiIvcG9zdGdyZXMgLUQgIiRQR0RBVEEiICIke1BBUkFNU1tAXX0iICY+ICIkUEdMT0ci
ICYKCiMgd2FpdCBmb3IgdGhlIHN0YXJ0IHRvIGNvbXBsZXRlCnVudGlsICIkUEdCSU4iL3BnX2lz
cmVhZHkgLXE7IGRvIHNsZWVwIC41OyBkb25lCgoiJFBHQklOIi9wZ2JlbmNoIC1xaXMgNQoKbHMg
IiR7UEdEQVRBfSIvcGdfd2FsL2FyY2hpdmVfc3RhdHVzCnBraWxsIC1TSUdBQlJUIC1mICJub24t
YXJjaC13YWwtb24tcmVjb3Zlcnk6IHdhbHdyaXRlciIKCndoaWxlICEgZ3JlcCAnd3JpdGUtYWhl
YWQgbG9nIGZpbGUnICIkUEdMT0ciIDsgZG8gc2xlZXAgMTsgZG9uZQoKIyBzdG9wIGNsdXN0ZXIg
aW4gYmFja2dyb3VuZCAKa2lsbCAlMQp3YWl0ICUxCg==

--MP_/WC6WEc8gEbd3qV4j9CEjgao
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0001-Fix-WAL-retention-during-production-crash-recovery.patch