Re: [HACKERS] WIP: long transactions on hot standby feedback replica / proof of concept

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Ivan Kartyshov <i(dot)kartyshov(at)postgrespro(dot)ru>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] WIP: long transactions on hot standby feedback replica / proof of concept
Date: 2018-08-12 01:11:14
Message-ID: CAPpHfdvHjXJKhQKD0-wyBA=V4s1Q9p5VVT9W8_bB-TGJXAph3w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi!

On Fri, Aug 10, 2018 at 5:07 PM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Thu, Aug 9, 2018 at 11:26 PM Ivan Kartyshov
> <i(dot)kartyshov(at)postgrespro(dot)ru> wrote:
> > Alexander Korotkov писал 2018-06-20 20:54:
> > > Thinking about that more I found that adding vacuum mark as an extra
> > > argument to LockAcquireExtended is also wrong. It would be still hard
> > > to determine if we should log the lock in LogStandbySnapshot(). We'll
> > > have to add that flag to shared memory table of locks. And that looks
> > > like a kludge.
> > >
> > > Therefore, I'd like to propose another approach: introduce new lock
> > > level. So, AccessExclusiveLock will be split into two
> > > AccessExclusiveLocalLock and AccessExclusiveLock. In spite of
> > > AccessExclusiveLock, AccessExclusiveLocalLock will be not logged to
> > > standby, and used for heap truncation.
> > >
> > > I expect some resistance to my proposal, because fixing this "small
> > > bug" doesn't deserve new lock level. But current behavior of
> > > hot_standby_feedback is buggy. From user prospective,
> > > hot_standby_feedback option is just non-worker, which causes master to
> > > bloat without protection for standby queries from cancel. We need to
> > > fix that, but I don't see other proper way to do that except adding
> > > new lock level...
> >
> > Your offer is very interesting, it made patch smaller and more
> > beautiful.
> > So I update patch and made changes accordance with the proposed concept
> > of
> > special AccessExclusiveLocalLock.
>
> > I would like to here you opinion over this implementation.
>
> In general this patch looks good for me. It seems that comments and
> documentation need minor enhancements. I'll make them and post the
> revised patch.

Find the revised patch attached. It contains some enhancements in
comments, formatting and documentation. BTW, I decided that we should
enumerate ACCESS EXCLUSIVE LOCAL lock before ACCESS EXCLUSIVE lock,
because we enumerate lock on ascending strength. So, since ACCESS
EXCLUSIVE is WAL-logged, it's definitely "stronger".

I think that introduction of new lock level is significant change and
can't be backpatched. But I think it worth to backpatch a note to the
documentation, which clarifies why hot_standby_feedback might have
unexpected behavior.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
hsfeedback_locallock-2.patch application/octet-stream 20.0 KB
hsfeedback_backpatch_note.patch application/octet-stream 819 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-08-12 02:23:29 Re: Allowing printf("%m") only where it actually works
Previous Message Andrew Dunstan 2018-08-11 23:32:51 Re: NetBSD vs libxml2