From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Peter Geoghegan <pg(at)bowt(dot)ie>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: ReadRecentBuffer() doesn't scale well |
Date: | 2023-06-29 07:35:30 |
Message-ID: | CA+hUKGLMFtNqei9nfcJy2SQMLWyYuO9E8NLYrb=4Gs1HgkAS7Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I (re)discovered why I used the lock-then-pin approach. In the
comments I mentioned InvalidBuffer(), but the main problem is in its
caller GetVictimBuffer() which has various sanity checks about
reference counts that can occasionally fail if you have code randomly
pinning any old buffer.
New idea: use the standard PinBuffer() function, but add a mode that
doesn't pin invalid buffers (with caveat that you can perhaps get a
false negative due to unlocked read, but never a false positive; see
commit message). Otherwise we'd have to duplicate all the same logic
to use cmpxchg for ReadRecentBuffer(), or rethink the assumptions in
that other code.
As for the lack of usage bump in the back-branches, I think the
options are: teach PinBuffer_Locked() to increment it optionally, or
back-patch whatever we come up with for this.
For the root buffer optimisation, the obvious place for storage seems
to be under rd_amcache. It was originally invented for the cached
metapage (commit d2896a9ed14) but could accommodate a new struct
holding whatever we want. Here is a patch to try that out.
BTAMCacheData would also be a natural place to put future things
including a copy of the root page itself, in later work on lock-free
tricks.
Experimental patches attached.
Attachment | Content-Type | Size |
---|---|---|
0001-Improve-ReadRecentBuffer-scalability.patch | text/x-patch | 5.8 KB |
0002-Use-ReadRecentBuffer-for-btree-root-page.patch | text/x-patch | 7.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Richard Guo | 2023-06-29 07:39:38 | Re: Assert !bms_overlap(joinrel->relids, required_outer) |
Previous Message | Heikki Linnakangas | 2023-06-29 07:06:35 | Re: Add more sanity checks around callers of changeDependencyFor() |