Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: MultiXact\SLRU buffers configuration
From: Andrey Borodin <x4mmm@yandex-team.ru>
In-Reply-To: 
 <CA+hUKGJubZLN16cTJjaeEpWdaTsKYuLaV67N+0Mps521zYo6ig@mail.gmail.com>
Date: Mon, 29 Mar 2021 13:26:02 +0500
Cc: Gilles Darold <gilles@darold.net>,
 Tomas Vondra <tomas.vondra@enterprisedb.com>,
 Tomas Vondra <tomas.vondra@2ndquadrant.com>,
 Alexander Korotkov <aekorotkov@gmail.com>,
 Anastasia Lubennikova <a.lubennikova@postgrespro.ru>,
 Daniel Gustafsson <daniel@yesql.se>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <018EF641-FC9D-4BA9-A458-6842E27E91C2@yandex-team.ru>
References: <a7f1c4e1-1015-92a4-2bd4-6736bd13d03e@postgrespro.ru>
 <20201028013651.de5cj2xadgmba5nf@development>
 <13D8FD63-559A-4737-B7FD-05288D1CEF8B@yandex-team.ru>
 <20201028233243.ygm6yqlynkqpzekr@development>
 <43F3DE92-F236-4EA5-B4D6-39BEF6BD849D@yandex-team.ru>
 <20201029134933.xd4mh2cofuf6tdfz@development>
 <65C1B4BA-D16F-4939-978B-AC8F370F5A5E@yandex-team.ru>
 <9b4d17df-b811-8323-16be-3cab913216d1@enterprisedb.com>
 <A64B924A-B08E-417D-BD3D-D396D722053B@yandex-team.ru>
 <35862787-8b4d-a290-789e-6e12dc6527e8@enterprisedb.com>
 <13F86913-C01B-4983-AE2E-493F5A028280@yandex-team.ru>
 <6ba7eae2-8b0c-0690-11a5-e921e6586180@darold.net>
 <3F5E23A3-6250-486E-9CCA-E2A5B6A28B4F@yandex-team.ru>
 <636f694a-4d69-d6d9-78d1-c88714f3e2e0@darold.net>
 <3319917a-679e-b07d-b194-473552b72082@darold.net>
 <d6f17699-0c18-1a47-8dfb-f52b36fb7c4f@darold.net>
 <D2CC4D64-CA89-411B-96D7-F0A4BF4BB895@yandex-team.ru>
 <b4911e88-9969-aaba-f6be-ed57bd5fec36@darold.net>
 <1E3DD5BF-A54E-4203-86C5-94C9EFA0E095@yandex-team.ru>
 <04ae150b-6442-148a-bb23-77082f6fca1d@darold.net>
 <3768803E-4794-409D-88D2-C3EF7CA1B7FC@yandex-team.ru>
 <CA+hUKGJ4TwipnMDmKdORu4zUo_rofsuu5FQsgV=UHJ3bY38ERw@mail.gmail.com>
 <CA+hUKGK2bkx91=CWRuPqWdXAu2JSS0mDPBh8DNuSeKhy8eOLgg@mail.gmail.com>
 <CA+hUKGL0g+Nmf=2XePaXtw82BkW9CRzoxxXB4gqZOk7tHxNJXg@mail.gmail.com>
 <D330C01E-789A-4DDD-B2B1-4561B8B7F41D@yandex-team.ru>
 <75B95FDD-2BE0-4525-8BDD-FC67C8BCBD53@yandex-team.ru>
 <CA+hUKGKVqrxOp82zER1=XN=yPwV_-OCGAg=ez=1iz9rG+A7Smw@mail.gmail.com>
 <88CB7042-6181-4EF4-8B25-61EC5E5725D4@yandex-team.ru>
 <CA+hUKGJubZLN16cTJjaeEpWdaTsKYuLaV67N+0Mps521zYo6ig@mail.gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/018EF641-FC9D-4BA9-A458-6842E27E91C2%40yandex-team.ru>
Precedence: bulk


> 29 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2021 =D0=B3., =D0=B2 02:15, Thomas =
Munro <thomas.munro@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=
=D0=B0):
>=20
> On Sat, Mar 27, 2021 at 6:31 PM Andrey Borodin <x4mmm@yandex-team.ru> =
wrote:
>>> 27 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2021 =D0=B3., =D0=B2 01:26, Thomas =
Munro <thomas.munro@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=
=D0=B0):
>>> , and murmurhash which is inlineable and
>>> branch-free.
>=20
>> I think pageno is a hash already. Why hash any further? And pages =
accessed together will have smaller access time due to colocation.
>=20
> Yeah, if clog_buffers is large enough then it's already a "perfect
> hash", but if it's not then you might get some weird "harmonic"
> effects (not sure if that's the right word), basically higher or lower
> collision rate depending on coincidences in the data.  If you apply a
> hash, the collisions should be evenly spread out so at least it'll be
> somewhat consistent.  Does that make sense?
As far as I understand "Harmonic" effects only make sense if the =
distribution is unknown. Hash protects from "periodic" data when periods =
are equal to hash table size. I don't think we need to protect from this =
case, SLRU data is expected to be localised...
Cost of this protection is necessity to calculate murmur hash on each =
SLRU lookup. Probably, 10-100ns. Seems like not a big deal.

> (At some point I figured out that the syscaches have lower collision
> rates and perform better if you use oids directly instead of hashing
> them... but then it's easy to create a pathological pattern of DDL
> that turns your hash table into a linked list.  Not sure what to think
> about that.)
>=20
>>> I had to tweak it to support "in-place" creation and
>>> fixed size (in other words, no allocators, for use in shared =
memory).
>=20
>> We really need to have a test to know what happens when this =
structure goes out of memory, as you mentioned below. What would be =
apropriate place for simplehash tests?
>=20
> Good questions.  This has to be based on being guaranteed to have
> enough space for all of the entries, so the question is really just
> "how bad can performance get with different load factors".  FWIW there
> were some interesting cases with clustering when simplehash was first
> used in the executor (see commits ab9f2c42 and parent) which required
> some work on hashing quality to fix.
Interesting read, I didn't know much about simple hash, but seems like =
there is still many cases where it can be used for good. I always =
wondered why Postgres uses only Larson's linear hash.

>=20
>>> Then I was annoyed that I had to add a "status" member to our =
struct,
>>> so I tried to fix that.
>=20
>> Indeed, sizeof(SlruMappingTableEntry) =3D=3D 9 seems strange. Will =
simplehash align it well?
>=20
> With that "intrusive status" patch, the size is back to 8.  But I
> think I made a mistake: I made it steal some key space to indicate
> presence, but I think the presence test should really get access to
> the whole entry so that you can encode it in more ways.  For example,
> with slotno =3D=3D -1.
>=20
> Alright, considering the date, if we want to get this into PostgreSQL
> 14 it's time to make some decisions.
>=20
> 1.  Do we want customisable SLRU sizes in PG14?
>=20
> +1 from me, we have multiple reports of performance gains from
> increasing various different SLRUs, and it's easy to find workloads
> that go faster.
Yes, this is main point of this discussion. So +1 from me too.

>=20
> One thought: it'd be nice if the user could *see* the current size,
> when using the default.  SHOW clog_buffers -> 0 isn't very helpful if
> you want to increase it, but don't know what it's currently set to.
> Not sure off the top of my head how best to do that.
Don't we expect that SHOW command indicate exactly same value as in =
config or SET command? If this convention does not exist - probably =
showing effective value is a good idea.


> 2.  What names do we want the GUCs to have?  Here's what we have:
>=20
> Proposed GUC              Directory            System views
> clog_buffers              pg_xact              Xact
> multixact_offsets_buffers pg_multixact/offsets MultiXactOffset
> multixact_members_buffers pg_multixact/members MultiXactMember
> notify_buffers            pg_notify            Notify
> serial_buffers            pg_serial            Serial
> subtrans_buffers          pg_subtrans          Subtrans
> commit_ts_buffers         pg_commit_ts         CommitTs
>=20
> By system views, I mean pg_stat_slru, pg_shmem_allocations and
> pg_stat_activity (lock names add "SLRU" on the end).
>=20
> Observations:
>=20
> It seems obvious that "clog_buffers" should be renamed to =
"xact_buffers".
+1
> It's not clear whether the multixact GUCs should have the extra "s"
> like the directories, or not, like the system views.
I think we show break the ties by native English speaker's ears or =
typing habits. I'm not a native speaker.

> It see that we have "Shared Buffer Lookup Table" in
> pg_shmem_allocations, so where I generated names like "Subtrans
> Mapping Table" I should change that to "Lookup" to match.
>=20
> 3.  What recommendations should we make about how to set it?
>=20
> I think the answer depends partially on the next questions!  I think
> we should probably at least say something short about the pg_stat_slru
> view (cache miss rate) and pg_stat_actitity view (waits on locks), and
> how to tell if you might need to increase it.  I think this probably
> needs a new paragraph, separate from the docs for the individual GUC.
I can only suggest incident-driven approach.
1. Observe ridiculous amount of backends waiting on particular SLRU.
2. Double SLRU buffers for that SLRU.
3. Goto 1.
I don't think we should mention this approach in docs.

> 4.  Do we want to ship the dynahash patch?

This patch allows to throw infinite amount of memory on a problem of =
SLRU waiting for IO. So the scale of improvement is much higher. Do I =
want that we ship this patch? Definitely. Does this change much? I don't =
know.

>=20
> +0.9.  The slight hesitation is that it's new code written very late
> in the cycle, so it may still have bugs or unintended consequences,
> and as you said, at small sizes the linear search must be faster than
> the hash computation.  Could you help test it, and try to break it?
I'll test it and try to break.

> Can we quantify the scaling effect for some interesting workloads, to
> see at what size the dynahash beats the linear search, so that we can
> make an informed decision?
I think we cannot statistically distinguish linear search from hash =
search by means of SLRU. But we can create some synthetic benchmarks.

> Of course, without a hash table, large
> sizes will surely work badly, so it'd be tempting to restrict the size
> you can set the GUC to.
>=20
> If we do include the dynahash patch, then I think it would also be
> reasonable to change the formula for the default, to make it higher on
> large systems.  The restriction to 128 buffers (=3D 1MB) doesn't make
> much sense on a high frequency OLTP system with 128GB of shared
> buffers or even 4GB.  I think "unleashing better defaults" would
> actually be bigger news than the GUC for typical users, because
> they'll just see PG14 use a few extra MB and go faster without having
> to learn about these obscure new settings.
I agree. I don't see why we would need to limit buffers to 128 in =
presence of hash search.

> 5.  Do we want to ship the simplehash patch?
>=20
> -0.5.  It's a bit too exciting for the last minute, so I'd be inclined
> to wait until the next cycle to do some more research and testing.  I
> know it's a better idea in the long run.
OK, obviously, it's safer decision.


My TODO list:
1. Try to break patch set v13-[0001-0004]
2. Think how to measure performance of linear search versus hash search =
in SLRU buffer mapping.

Thanks!

Best regards, Andrey Borodin.=