Hi Bertrand-san, > As f19c0ecca introduced online enabling and disabling of data checksums, I > think that it would make sense to do the same for wal_log_hints.
+1 on the general direction. > The pg_rewind check on ControlFile->wal_log_hints (which reflects > the current state at rewind time) remains sufficient. I'm not sure this holds. In fact, [1] seems to say the opposite: > It does not really matter if the node used log_hint_bits set to > false in its latest state (Node to-be-rewinded might have been > restarted after WAL forked). What pg_rewind actually requires is that the effective value was continuously on from the divergence point to the end of the target's WAL. pg_control only keeps the latest value, so an off period after the divergence goes undetected: 1. Run with wal_log_hints = off (data checksums off). 2. The standby is promoted (divergence). 3. On the old primary, a hint-bit-only page update happens (not WAL-logged). 4. wal_log_hints is set to on via reload (pg_control now says on). 5. The old primary is cleanly shut down and pg_rewind is run. The check passes, but the page from step 3 never shows up in extractPageMap() and is not copied from the source. Combined with the source's CLOG, the leftover hint bit yields incorrect tuple visibility. I was able to reproduce this: with a transaction held open across the divergence (insert before the last common checkpoint, commit after the divergence), the rewound node shows a row that does not exist on the source, due to a leftover HEAP_XMIN_COMMITTED hint. I can provide this as a TAP test. Note that 012_wal_log_hints.pl in the patch exercises exactly this sequence (diverge while off, reload to on, pg_rewind succeeds) -- the command succeeds, but the result is not consistent. Strictly speaking, a similar gap can already be created today by restarting with a different value, but a reload makes it much easier to hit. How about making pg_rewind fail if the WAL range it already scans in extractPageMap() contains an XLOG_PARAMETER_CHANGE record with wal_log_hints = off? Current pg_control saying on, plus no change record in the scanned range, guarantees the value was on for the whole range. This is essentially the tracking that [1] already suggested on the pg_rewind side, which does not seem to have been implemented. [1] https://postgr.es/m/cab7npqsxys4jg-kjmy8xim4stqkgkvhydrcoojhxzrskiw5...@mail.gmail.com Regards, Tatsuya Kawata
