|From:||Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>|
|To:||John Naylor <john(dot)naylor(at)2ndquadrant(dot)com>|
|Cc:||Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: WIP: Avoid creation of the free space map for small tables|
|Views:||Raw Message | Whole Thread | Download mbox|
On Fri, Jan 11, 2019 at 3:54 AM John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
> On Wed, Jan 9, 2019 at 10:50 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > Thanks, Mithun for performance testing, it really helps us to choose
> > the right strategy here. Once John provides next version, it would be
> > good to see the results of regular pgbench (read-write) runs (say at
> > 50 and 300 scale factor) and the results of large copy. I don't think
> > there will be any problem, but we should just double check that.
> Attached is v12 using the alternating-page strategy. I've updated the
> comments and README as needed. In addition, I've
> Any pages with wasted free space become visible at next relation extension, so we still control table bloat.
I think the free space will be available after the next pass of
vacuum, no? How can relation extension make it available?
+2. For very small heap relations, the FSM would be relatively large and
+wasteful, so as of PostgreSQL 12 we refrain from creating the FSM for
+heaps with HEAP_FSM_CREATION_THRESHOLD pages or fewer, both to save space
+and to improve performance. To locate free space in this case, we simply
+iterate over the heap, trying alternating pages in turn. There may be some
+wasted free space in this case, but it becomes visible again upon next
a. Again, how space becomes available at next relation extension.
b. I think there is no use of mentioning the version number in the
above comment, this code will be present from PG-12, so one can find
out from which version this optimization is added.
RecordAndGetPageWithFreeSpace(Relation rel, BlockNumber oldPage,
Size oldSpaceAvail, Size spaceNeeded)
+ /* First try the local map, if it exists. */
+ if (oldPage < fsm_local_map.nblocks)
The comment doesn't appear to be completely in sync with the code.
Can't we just check whether "fsm_local_map.nblocks > 0", if so, we
can use a macro for the same? I have changed this in the attached
patch, see what you think about it. I have used it at a few other
places as well.
+ * When we initialize the map, the whole heap is potentially available to
+ * try. If a caller wanted to reset the map after another backend extends
+ * the relation, this will only flag new blocks as available. No callers
+ * do this currently, however.
+fsm_local_set(Relation rel, BlockNumber curr_nblocks)
+ if (blkno >= fsm_local_map.nblocks + 2)
The way you have tried to support the case as quoted in the comment
"If a caller wanted to reset the map after another backend extends .."
doesn't appear to be solid and I am not sure if it is correct either.
We don't have any way to test the same, so I suggest let's try to
simplify the case w.r.t current requirement of this API. I think we
some simple logic to try every other block like:
+ blkno = cur_nblocks - 1;
+ while (true)
+ fsm_local_map.map[blkno] = FSM_LOCAL_AVAIL;
+ if (blkno >= 2)
+ blkno -= 2;
I have changed this in the attached patch.
+ * Search the local map for an available block to try, in descending order.
+ * For use when there is no FSM.
We should give a brief explanation as to why we try in descending
order. I have added some explanation in the attached patch, see what
you think about it?
Apart from the above, I have modified a few comments.
|Next Message||Amit Kapila||2019-01-16 13:45:05||Re: WIP: Avoid creation of the free space map for small tables|
|Previous Message||Daniel Verite||2019-01-16 13:20:50||Re: insensitive collations|