MIME-Version: 1.0
References: <CAFiTN-sTRFEzSZxGHoOopdq+gTcOA97qiF=SckmBtYYFtsm-Mw@mail.gmail.com>
 <202311160941.olx5z3a7pkww@alvherre.pgsql>
In-Reply-To: <202311160941.olx5z3a7pkww@alvherre.pgsql>
From: Dilip Kumar <dilipbalaut@gmail.com>
Date: Fri, 17 Nov 2023 13:09:03 +0530
Message-ID: <CAFiTN-tpqOXHTrMhhaBZKr2Y6vkr0vgysFQt0B_EDqipDeNGog@mail.gmail.com>
Subject: Re: SLRU optimization - configurable buffer pool and partitioning the
 SLRU lock
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAFiTN-tpqOXHTrMhhaBZKr2Y6vkr0vgysFQt0B_EDqipDeNGog%40mail.gmail.com>
Precedence: bulk

On Thu, Nov 16, 2023 at 3:11=E2=80=AFPM Alvaro Herrera <alvherre@alvh.no-ip=
.org> wrote:
>
> I just noticed that 0003 does some changes to
> TransactionGroupUpdateXidStatus() that haven't been adequately
> explained AFAICS.  How do you know that these changes are safe?

IMHO this is safe as well as logical to do w.r.t. performance.  It's
safe because whenever we are updating any page in a group we are
acquiring the respective bank lock in exclusive mode and in extreme
cases if there are pages from different banks then we do switch the
lock as well before updating the pages from different groups.  And, we
do not wake any process in a group unless we have done the status
update for all the processes so there could not be any race condition
as well.  Also, It should not affect the performance adversely as well
and this will not remove the need for group updates.  The main use
case of group update is that it will optimize a situation when most of
the processes are contending for status updates on the same page and
processes that are waiting for status updates on different pages will
go to different groups w.r.t. that page, so in short in a group on
best effort basis we are trying to have the processes which are
waiting to update the same clog page that mean logically all the
processes in the group will be waiting on the same bank lock.  In an
extreme situation if there are processes in the group that are trying
to update different pages or even pages from different banks then we
are handling it well by changing the lock.  Although someone may raise
a concern that in cases where there are processes that are waiting for
different bank locks then after releasing one lock why not wake up
those processes, I think that is not required because that is the
situation we are trying to avoid where there are processes trying to
update different are in the same group so there is no point in adding
complexity to optimize that case.


> 0001 contains one typo in the docs, "cotents".
>
> I'm not a fan of the fact that some CLOG sizing macros moved to clog.h,
> leaving others in clog.c.  Maybe add commentary cross-linking both.
> Alternatively, perhaps allowing xact_buffers to grow beyond 65536 up to
> the slru.h-defined limit of 131072 is not that bad, even if it's more
> than could possibly be needed for xact_buffers; nobody is going to use
> 64k buffers, since useful values are below a couple thousand anyhow.

I agree, that allowing xact_buffers to grow beyond 65536 up to the
slru.h-defined limit of 131072 is not that bad, so I will change that
in the next version.

--=20
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com