Re: Reviewing freeze map code

On Tue, Jun 21, 2016 at 9:21 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> On 2016-06-21 08:59:13 +0530, Amit Kapila wrote:
> > Can we consider to use some strategy to avoid deadlocks without
releasing
> > the lock on old page? Consider if we could have a mechanism such that
> > RelationGetBufferForTuple() will ensure that it always returns a new
buffer
> > which has targetblock greater than the old block (on which we already
held
> > a lock). I think here tricky part is whether we can get anything like
that
> > from FSM. Also, there could be cases where we need to extend the heap
when
> > there were pages in heap with space available, but we have ignored them
> > because there block number is smaller than the block number on which we
> > have lock.
>
> I can't see that being acceptable, from a space-usage POV.
>
> > > So far the best idea I have - and it's really not a good one - is to
> > > invent a new hint-bit that tells concurrent updates to acquire a
> > > heavyweight tuple lock, while releasing the page-level lock. If that
> > > hint bit does not require any other modifications - and we don't need
an
> > > xid in xmax for this use case - that'll avoid doing all the other
> > > `already_marked` stuff early, which should address the correctness
> > > issue.
> > >
> >
> > Don't we need to clear such a flag in case of error? Also don't we
need to
> > reset it later, like when modifying the old page later before WAL.
>
> If the flag just says "acquire a heavyweight lock", then there's no need
> for that. That's cheap enough to just do if it's errorneously set. At
> least I can't see any reason.
>

I think it will just increase the chances of other backends to acquire a
heavy weight lock.

> > > It's kinda invasive though, and probably has performance
> > > implications.
> > >
> >
> > Do you see performance implication due to requirement of heavywieht
tuple
> > lock in more cases than now or something else?
>
> Because of that, yes.
>
>
> > Some others ways could be:
> >
> > Before releasing the lock on buffer containing old tuple, clear the VM
and
> > visibility info from page and WAL log it. I think this could impact
> > performance depending on how frequently we need to perform this action.
>
> Doubling the number of xlog inserts in heap_update would certainly be
> measurable :(. My guess is that the heavyweight tuple lock approach will
> be less expensive.
>

Probably, but I think heavyweight tuple lock is more invasive. I think
increasing the number of xlog inserts could surely impact performance, but
depending upon how frequently we need to call it. I think we might want to
combine it with the idea of RelationGetBufferForTuple() to return
higher-block number, such that if we don't find higher block-number from
FSM, then we can release the lock on old page and try to get the locks on
old and new buffers as we do now. This will further reduce the chances of
increasing xlog insert calls and address the issue of space-wastage.

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