Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
Subject: Re: MultiXact\SLRU buffers configuration
From: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
In-Reply-To: <20200514.151659.1224056315528271804.horikyota.ntt@gmail.com>
Date: Thu, 14 May 2020 11:44:01 +0500
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <102F673B-C9C5-4823-A92F-96799FC61ED0@yandex-team.ru>
References: <FBFC01AF-9F5B-469B-B8A3-627B89684ADB@yandex-team.ru>
 <20200514.102526.1602501255796880628.horikyota.ntt@gmail.com>
 <3B099683-ECCD-43CD-A3D6-F08C3745002A@yandex-team.ru>
 <20200514.151659.1224056315528271804.horikyota.ntt@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Precedence: bulk


> 14 =D0=BC=D0=B0=D1=8F 2020 =D0=B3., =D0=B2 11:16, Kyotaro Horiguchi =
<horikyota.ntt@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0=
):
>=20
> At Thu, 14 May 2020 10:19:42 +0500, "Andrey M. Borodin" =
<x4mmm@yandex-team.ru> wrote in=20
>>>> I'm looking more at MultiXact and it seems to me that we have a =
race condition there.
>>>>=20
>>>> When we create a new MultiXact we do:
>>>> 1. Generate new MultiXactId under MultiXactGenLock
>>>> 2. Record new mxid with members and offset to WAL
>>>> 3. Write offset to SLRU under MultiXactOffsetControlLock
>>>> 4. Write members to SLRU under MultiXactMemberControlLock
>>>=20
>>> But, don't we hold exclusive lock on the buffer through all the =
steps
>>> above?
>> Yes...Unless MultiXact is observed on StandBy. This could lead to =
observing inconsistent snapshot: one of lockers committed tuple delete, =
but standby sees it as alive.
>=20
> Ah, right. I looked from GetNewMultiXactId. Actually
> XLOG_MULTIXACT_CREATE_ID is not protected from concurrent reference to
> the creating mxact id. And GetMultiXactIdMembers is considering that
> case.
>=20
>>>> When we read MultiXact we do:
>>>> 1. Retrieve offset by mxid from SLRU under =
MultiXactOffsetControlLock
>>>> 2. If offset is 0 - it's not filled in at step 4 of previous =
algorithm, we sleep and goto 1
>>>> 3. Retrieve members from SLRU under MultiXactMemberControlLock
>>>> 4. ..... what we do if there are just zeroes because step 4 is not =
executed yet? Nothing, return empty members list.
>>>=20
>>> So transactions never see such incomplete mxids, I believe.
>> I've observed sleep in step 2. I believe it's possible to observe =
special effects of step 4 too.
>> Maybe we could add lock on standby to dismiss this 1000us wait? =
Sometimes it hits hard on Standbys: if someone is locking whole table on =
primary - all seq scans on standbys follow him with =
MultiXactOffsetControlLock contention.
>=20
> GetMultiXactIdMembers believes that 4 is successfully done if 2
> returned valid offset, but actually that is not obvious.
>=20
> If we add a single giant lock just to isolate ,say,
> GetMultiXactIdMember and RecordNewMultiXact, it reduces concurrency
> unnecessarily.  Perhaps we need finer-grained locking-key for standby
> that works similary to buffer lock on primary, that doesn't cause
> confilicts between irrelevant mxids.
>=20
We can just replay members before offsets. If offset is already there - =
members are there too.
But I'd be happy if we could mitigate those 1000us too - with a hint =
about last maixd state in a shared MX state, for example.

Actually, if we read empty mxid array instead of something that is =
replayed just yet - it's not a problem of inconsistency, because =
transaction in this mxid could not commit before we started. ISTM.
So instead of fix, we, probably, can just add a comment. If this =
reasoning is correct.

Best regards, Andrey Borodin.=