Re: WIP: Avoid creation of the free space map for small tables

On Mon, Dec 3, 2018 at 11:15 AM John Naylor <jcnaylor(at)gmail(dot)com> wrote:
>
> On 12/3/18, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Mon, Dec 3, 2018 at 9:46 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >>
> >> On Thu, Nov 29, 2018 at 3:07 PM John Naylor <jcnaylor(at)gmail(dot)com> wrote:
> >> >
> >>
> > v8 code:
> > +fsm_local_set(Relation rel, BlockNumber new_nblocks)
> > +{
> > + BlockNumber blkno,
> > + cached_target_block;
> > +
> > + /*
> > + * Mark blocks available starting after the last block number we have
> > + * cached, and ending at the current last block in the relation.
> > + * When we first set the map, this will flag all blocks as available
> > + * to try. If we reset the map while waiting for a relation
> > + * extension lock, this will only flag new blocks as available,
> > + * if any were created by another backend.
> > + */
> > + for (blkno = fsm_local_map.nblocks; blkno < new_nblocks; blkno++)
> > + fsm_local_map.map[blkno] = FSM_LOCAL_AVAIL;
> >
> > v9 code:
> > +static void
> > +fsm_local_set(Relation rel, BlockNumber nblocks)
> > +{
> > + BlockNumber blkno,
> > + cached_target_block;
> > +
> > + for (blkno = 0; blkno < nblocks; blkno++)
> > + fsm_local_map.map[blkno] = FSM_LOCAL_AVAIL;
> >
> > What is the reason for the above code change in the latest patch version?
>
> Per your recent comment, we no longer check relation size if we waited
> on a relation extension lock, so this is essentially a reversion to an
> earlier version.
>

fsm_local_set is being called from RecordAndGetPageWithFreeSpace and
GetPageWithFreeSpace whereas the change we have discussed was specific
to GetPageWithFreeSpace, so not sure if we need any change in
fsm_local_set.

> Keeping v8 would have the advantage that it'd be
> simple to change our minds about this. Do you have an opinion about
> that?
>
> > It would be good if you add few comments atop functions
> > GetPageWithFreeSpace, RecordAndGetPageWithFreeSpace and
> > RecordPageWithFreeSpace about their interaction with local map.
>
> Good idea, will do.
>

Thanks.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com