Re: Reviewing freeze map code - Mailing list pgsql-hackers
| From | Amit Kapila |
|---|---|
| Subject | Re: Reviewing freeze map code |
| Date | |
| Msg-id | CAA4eK1LAjMm=jHCrfvnuLHvDLXAXZjY1X1YdzhQF3m6WiQKS+g@mail.gmail.com Whole thread Raw |
| In response to | Re: Reviewing freeze map code (Andres Freund <andres@anarazel.de>) |
| List | pgsql-hackers |
On Tue, Jun 21, 2016 at 9:21 AM, Andres Freund <andres@anarazel.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.
>
> > > 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.
>
>
> 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.
pgsql-hackers by date: