On Wed, 27 Jul 2022 at 08:08, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> At Tue, 26 Jul 2022 23:09:16 +0500, Andrey Borodin <x4mmm@yandex-team.ru> wrote in
> >
> >
> > > On 20 Jul 2022, at 02:12, Michail Nikolaev <michail.nikolaev@gmail.com> wrote:
> > >
> > >> I've looked into v5.
> > > Thanks!
> > >
> > > Patch is updated accordingly your remarks.
> >
> > The patch seems Ready for Committer from my POV.
>
> + * is s updated during taking the snapshot. The KnownAssignedXidsNextOffset
> + * contains not an offset to the next valid xid but a number which tends to
> + * the offset to next valid xid. KnownAssignedXidsNextOffset[] values read
>
> Is there still a reason why the array stores the distnace to the next
> valid element instead of the index number of the next valid index? It
> seems to me that that was in an intention to reduce the size of the
> offset array but it is int32[] which is far wider than
> TOTAL_MAX_CACHED_SUBXIDS.
>
> It seems to me storing the index itself is simpler and maybe faster by
> the cycles to perform addition.
I'm inclined to think this is all too much. All of this optimization
makes sense when the array spreads out horribly, but we should be just
avoiding that in the first place by compressing more often.
The original coded frequency of compression was just a guess and was
never tuned later.
A simple patch like this seems to hit the main concern, aiming to keep
the array from spreading out and impacting snapshot performance for
SELECTs, yet not doing it so often that the startup process has a
higher burden of work.
--
Simon Riggs http://www.EnterpriseDB.com/