Re: Unhappy about API changes in the no-fsm-for-small-rels patch
John Naylor <john.naylor@2ndquadrant.com>
From: John Naylor <john.naylor@2ndquadrant.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-04-19T07:47:27Z
Lists: pgsql-hackers
On Fri, Apr 19, 2019 at 10:38 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Apr 18, 2019 at 2:10 PM John Naylor <john.naylor@2ndquadrant.com> wrote: > > Agreed. I suspect the most realistic way to address most of the > > objections in a short amount of time would be to: > > > > 1. rip out the local map > > 2. restore hio.c to only checking the last block in the relation if > > there is no FSM (and lower the threshold to reduce wasted space) > > 3. reduce calls to smgr_exists() > > > > Won't you need an extra call to RelationGetNumberofBlocks to find the > last block? If I understand you correctly, no, the call now in GetPageWithFreeSpace() just moves it back to where it was in v11. In the corner case where we just measured the table size and the last block is full, we can pass nblocks to RecordAndGetPageWithFreeSpace(). There might be further optimizations available if we're not creating a local map. > Also won't it be less efficient in terms of dealing with > bloat as compare to current patch? Yes. The threshold would have to be 2 or 3 blocks, and it would stay bloated until it passed the threshold. Not great, but perhaps not bad either. > I think if we go this route, then > we might need to revisit it in the future to optimize it, but maybe > that is the best alternative as of now. It's a much lighter-weight API, which has that much going for it. I have a draft implementation, which I can share if it comes to that -- it needs some more thought and polish first. > I am thinking that we should at least give it a try to move the map to > rel cache level to see how easy or difficult it is and also let's wait > for a day or two to see if Andres/Tom has to say anything about this > or on the response by me above to improve the current patch. Since we have a definite timeline, I'm okay with that, although I'm afraid I'm not quite knowledgeable enough to help much with the relcache piece. -- John Naylor https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Revert "Avoid the creation of the free space map for small heap relations".
- 7db0cde6b58e 12.0 landed
-
Improve code comments in b0eaa4c51b.
- 06c8a5090ed9 12.0 cited
-
During pg_upgrade, conditionally skip transfer of FSMs.
- 13e8643bfc29 12.0 cited
-
Add more tests for FSM.
- 6f918159a97a 12.0 cited
-
Fix typos in commit 8586bf7ed8.
- a6e48da08844 12.0 cited
-
tableam: introduce table AM infrastructure.
- 8586bf7ed888 12.0 cited
-
Clear the local map when not used.
- 9c32e4c35026 12.0 cited
-
Doc: Update the documentation for FSM behavior for small tables.
- 29d108cdecbe 12.0 cited
-
Make FSM test portable.
- 08ecdfe7e5e0 12.0 cited
-
Avoid creation of the free space map for small heap relations, take 2.
- b0eaa4c51bbf 12.0 cited