PG13 fails to startup even though the current transaction is equal to the target transaction

We've noticed what may be a regression / bug in PG13.

I work at Heroku on the Data team, where we manage a fleet of Postgres services. This change has resulted in breaking the UX we offer to customers to manage their PG services. In particular, ‘forks’ and ‘point in time restores’ seem broken for PG13.

I believe it is related to this patch (https://www.postgresql.org/message-id/993736dd3f1713ec1f63fc3b653839f5%40lako.no)

For PG12, we expect:

-- We create a new Postgres service from archive and provide a recovery_target_xid
-- PG replays the archive until the end of the archive is reached, and the current transaction == recovery_target_xid
-- We measure the current transaction via the query SELECT pg_catalog.txid_snapshot_xmax(pg_catalog.txid_current_snapshot())
-- Since the current transaction is exactly equal to the target transaction, we perform the promotion

For PG12, what we get:

-- This process completes smoothly, and the new postgres service is up and running

For PG13, we expect:

-- We create a new Postgres service from archive and provide a recovery_target_xid
-- PG replays the archive until the end of the archive is reached, and the current transaction == recovery_target_xid
-- We measure the current transaction via the query SELECT pg_catalog.txid_snapshot_xmax(pg_catalog.txid_current_snapshot())
-- Since the current transaction is exactly equal to the target transaction, we perform the promotion

For PG13, what we get:

-- On promotion we see the postgres process exit with the following log lines:

Mar 17 14:47:49 ip-10-0-146-54 25a9551c_65ec_4870_99e9_df69151984a0[7]: [18-1] sql_error_code = 00000 LOG: promote trigger file found: /etc/postgresql/wal-e.d/pull-env/STANDBY_OFF
Mar 17 14:47:49 ip-10-0-146-54 25a9551c_65ec_4870_99e9_df69151984a0[7]: [19-1] sql_error_code = 00000 LOG: redo done at 0/60527E0
Mar 17 14:47:49 ip-10-0-146-54 25a9551c_65ec_4870_99e9_df69151984a0[7]: [20-1] sql_error_code = 00000 LOG: last completed transaction was at log time 2021-03-17 14:42:44.901448+00
Mar 17 14:47:49 ip-10-0-146-54 25a9551c_65ec_4870_99e9_df69151984a0[7]: [21-1] sql_error_code = XX000 FATAL: recovery ended before configured recovery target was reached
Mar 17 14:47:49 ip-10-0-146-54 25a9551c_65ec_4870_99e9_df69151984a0[5]: [8-1] sql_error_code = 00000 LOG: startup process (PID 7) exited with exit code 1

Even though the transaction IDs are identical. It seems like the end of the archive was reached (in fact the last transaction), and while we arrived at the correct transaction id, somehow PG decided it wasn’t done replaying? Perhaps because somehow the timestamps don’t line up? (Afaict we do not set the recovery_target_time, just the recovery_target_xid)

We have the `recovery_target_inclusive` set to true, which is the default. It really seems like the intent of that setting means that if the target equals the current transaction ID, recovery should be marked as complete. However we're seeing the opposite. While the current txn id == the target transaction id, the server exits. This is surprising, and doesn't line up with our expected behavior.

We have a workaround.

Right before promotion, if we increment the transaction of the leader database (the original PG service that we're forking from) by running `SELECT pg_catalog.txid_current()`, wait 120 seconds (double our archive timeout value to allow for the WAL segment to be written / uploaded / read), and wait until the current transaction is strictly greater than the target transaction, then the promotion seems to work fine every time for PG13. But this seems like an off by one error?

What do you think? Is this a bug? Is this expected? Is this user error on our end?

Thanks!

Sean