On Wed, Sep 20, 2023 at 1:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I've tested this patch version on Debian 9 and it works fine now.
Progress...
> But as to wal_log_hints, I see that v7 patches work for REL_12_STABLE and
> REL_13_STABLE, but not for other branches, though v6 patches definitely
> failed on REL_12_STABLE and REL_13_STABLE too. So the behavior improved,
> but not for all versions, for some reason (I haven't looked deeper yet).
Ahh, yeah, there was a second problem screwing up the LSN accounting.
I was trying to write correct xl_prev links, but also emitting
optional padding records to get into the right position (depending on
initial conditions varying between branches etc), and I forgot about
the extra COMMIT records that emit_message() generates. Which, I
guess, comes back to Michael's observation that this would all be a
bit easier if we had a way to emit and flush a single record...
The solution in this version is to call get_insert_len() instead of
using the result of emit_message() for the values returned by the
advance_XXX() functions. The result of emit_message() is actually the
LSN of the following COMMIT record so can't be used directly for
building xl_prev chains.
> Also I've noticed a few minor discrepancies in the patches.
> A commit message in patch.15 contains the sentence "We should fix ...",
> not "We could fix ...", as in commit messages for other versions.
> Also, patch.12 contains a duplicated "Reviewed-by" string.
> And may be "xlp_prev" is a typo?
Fixed.