Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: tender wang <tndrwang@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
"Andrey M. Borodin" <x4mmm@yandex-team.ru>
Date: 2023-12-12T13:28:51Z
Lists: pgsql-hackers
[Added Andrey again in CC, because as I understand they are using this code or something like it in production. Please don't randomly remove people from CC lists.] I've been looking at this some more, and I'm not confident in that the group clog update stuff is correct. I think the injection points test case was good enough to discover a problem, but it's hard to get peace of mind that there aren't other, more subtle problems. The problem I see is that the group update mechanism is designed around contention of the global xact-SLRU control lock; it uses atomics to coordinate a single queue when the lock is contended. So if we split up the global SLRU control lock using banks, then multiple processes using different bank locks might not contend. OK, this is fine, but what happens if two separate groups of processes encounter contention on two different bank locks? I think they will both try to update the same queue, and coordinate access to that *using different bank locks*. I don't see how can this work correctly. I suspect the first part of that algorithm, where atomics are used to create the list without a lock, might work fine. But will each "leader" process, each of which is potentially using a different bank lock, coordinate correctly? Maybe this works correctly because only one process will find the queue head not empty? If this is what happens, then there needs to be comments about it. Without any explanation, this seems broken and potentially dangerous, as some transaction commit bits might become lost given high enough concurrency and bad luck. Maybe this can be made to work by having one more lwlock that we use solely to coordinate this task. Though we would have to demonstrate that coordinating this task with a different lock works correctly in conjunction with the per-bank lwlock usage in the regular slru.c paths. Andrey, do you have any stress tests or anything else that you used to gain confidence in this code? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "El sabio habla porque tiene algo que decir; el tonto, porque tiene que decir algo" (Platon).
Commits
-
Fix zeroing of pg_serial page without SLRU bank lock
- be2f07310063 17.0 landed
-
Fix misspelled assertions
- 0d3a71d0c8a7 17.0 landed
-
GUC table: Add description to computed variables
- 30b8d6e4ce11 17.0 landed
-
Improve performance of subsystems on top of SLRU
- 53c2a97a9266 17.0 landed
-
Rename SLRU elements in view pg_stat_slru
- bcdfa5f2e2f2 17.0 landed
-
Use atomic access for SlruShared->latest_page_number
- d172b717c6f4 17.0 landed
-
Split use of SerialSLRULock, creating SerialControlLock
- 7b745d85b80d 17.0 landed
-
Index SLRUs by 64-bit integers rather than by 32-bit integers
- 4ed8f0913bfd 17.0 cited
-
Add a macro templatized hashtable.
- b30d3ea824c5 10.0 cited