Re: Autovacuum cancellation

Lists: pgsql-committerspgsql-hackerspgsql-patches
From: alvherre(at)postgresql(dot)org (Alvaro Herrera)
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 14:45:55
Message-ID: 20071025144555.36432754229@cvs.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Log Message:
-----------
Extract catalog info for error reporting before an error actually happens.
Also, remove redundant reset of for-wraparound PGPROC flag.

Thanks to Tom Lane for noticing both bogosities.

Modified Files:
--------------
pgsql/src/backend/postmaster:
autovacuum.c (r1.63 -> r1.64)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/autovacuum.c?r1=1.63&r2=1.64)


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)postgresql(dot)org>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 15:54:23
Message-ID: 1193327663.4242.298.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Thu, 2007-10-25 at 14:45 +0000, Alvaro Herrera wrote:
> Log Message:
> -----------
> Extract catalog info for error reporting before an error actually happens.
> Also, remove redundant reset of for-wraparound PGPROC flag.

Just noticed you've made these changes. I was working on them, but
hadn't fully tested the patch because of all the different touch points.
Sorry for the delay.

Would you like me to refresh my earlier patch against the newly
committed state (or did you commit that aspect already as well?).

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 16:41:50
Message-ID: 20071025164150.GF23566@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Simon Riggs wrote:
> On Thu, 2007-10-25 at 14:45 +0000, Alvaro Herrera wrote:
> > Log Message:
> > -----------
> > Extract catalog info for error reporting before an error actually happens.
> > Also, remove redundant reset of for-wraparound PGPROC flag.
>
> Just noticed you've made these changes. I was working on them, but
> hadn't fully tested the patch because of all the different touch points.
> Sorry for the delay.
>
> Would you like me to refresh my earlier patch against the newly
> committed state (or did you commit that aspect already as well?).

I am doing just that. I'll post it soon.

FWIW I disagree with cancelling just any autovac work automatically; in
my patch I'm only cancelling if it's analyze, on the grounds that if
you have really bad luck you can potentially lose a lot of work that
vacuum did. We can relax this restriction when we have cancellable
vacuum.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
"La principal característica humana es la tontería"
(Augusto Monterroso)


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 16:56:30
Message-ID: 1193331390.4242.305.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Thu, 2007-10-25 at 13:41 -0300, Alvaro Herrera wrote:
> Simon Riggs wrote:
> > On Thu, 2007-10-25 at 14:45 +0000, Alvaro Herrera wrote:
> > > Log Message:
> > > -----------
> > > Extract catalog info for error reporting before an error actually happens.
> > > Also, remove redundant reset of for-wraparound PGPROC flag.
> >
> > Just noticed you've made these changes. I was working on them, but
> > hadn't fully tested the patch because of all the different touch points.
> > Sorry for the delay.
> >
> > Would you like me to refresh my earlier patch against the newly
> > committed state (or did you commit that aspect already as well?).
>
> I am doing just that. I'll post it soon.

OK thanks.

> FWIW I disagree with cancelling just any autovac work automatically; in
> my patch I'm only cancelling if it's analyze, on the grounds that if
> you have really bad luck you can potentially lose a lot of work that
> vacuum did. We can relax this restriction when we have cancellable
> vacuum.

That was requested by others, not myself, but I did agree with the
conclusions. The other bad luck might be that you don't complete some
critical piece of work in the available time window because an automated
job kicked in.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Michael Paesold <mpaesold(at)gmx(dot)at>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 17:09:08
Message-ID: 4720CDB4.6070501@gmx.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Simon Riggs wrote:
> On Thu, 2007-10-25 at 13:41 -0300, Alvaro Herrera wrote:
...
>> FWIW I disagree with cancelling just any autovac work automatically; in
>> my patch I'm only cancelling if it's analyze, on the grounds that if
>> you have really bad luck you can potentially lose a lot of work that
>> vacuum did. We can relax this restriction when we have cancellable
>> vacuum.
>
> That was requested by others, not myself, but I did agree with the
> conclusions. The other bad luck might be that you don't complete some
> critical piece of work in the available time window because an automated
> job kicked in.

Yeah, I thought we had agreed that we must cancel all auto
vacuum/analyzes, on the ground that foreground operations are usually
more important than maintenance tasks. Remember the complaint we already
had on hackers just after beta1: auto *vacuum* blocked a schema change,
and of course the user complained.

Best Regards
Michael Paesold


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Michael Paesold <mpaesold(at)gmx(dot)at>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 17:23:02
Message-ID: 20071025172301.GG23566@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Michael Paesold wrote:
> Simon Riggs wrote:
>> On Thu, 2007-10-25 at 13:41 -0300, Alvaro Herrera wrote:
> ...
>>> FWIW I disagree with cancelling just any autovac work automatically; in
>>> my patch I'm only cancelling if it's analyze, on the grounds that if
>>> you have really bad luck you can potentially lose a lot of work that
>>> vacuum did. We can relax this restriction when we have cancellable
>>> vacuum.
>> That was requested by others, not myself, but I did agree with the
>> conclusions. The other bad luck might be that you don't complete some
>> critical piece of work in the available time window because an automated
>> job kicked in.
>
> Yeah, I thought we had agreed that we must cancel all auto vacuum/analyzes,
> on the ground that foreground operations are usually more important than
> maintenance tasks.

What this means is that autovacuum will be starved a lot of the time,
and in the end you will only vacuum the tables when you run out of time
for Xid wraparound.

> Remember the complaint we already had on hackers just after beta1:
> auto *vacuum* blocked a schema change, and of course the user
> complained.

Actually I can't remember it, but I think we should decree that this is
a known shortcoming; that we will fix it when we have cancellable
vacuum; and that the user is free to cancel the vacuuming on his own if
he so decides.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
"The ability to monopolize a planet is insignificant
next to the power of the source"


From: Michael Paesold <mpaesold(at)gmx(dot)at>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 17:36:43
Message-ID: 4720D42B.4030601@gmx.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Alvaro Herrera wrote:
> Michael Paesold wrote:
>> Simon Riggs wrote:
>>> On Thu, 2007-10-25 at 13:41 -0300, Alvaro Herrera wrote:
>> ...
>>>> FWIW I disagree with cancelling just any autovac work automatically; in
>>>> my patch I'm only cancelling if it's analyze, on the grounds that if
>>>> you have really bad luck you can potentially lose a lot of work that
>>>> vacuum did. We can relax this restriction when we have cancellable
>>>> vacuum.
>>> That was requested by others, not myself, but I did agree with the
>>> conclusions. The other bad luck might be that you don't complete some
>>> critical piece of work in the available time window because an automated
>>> job kicked in.
>> Yeah, I thought we had agreed that we must cancel all auto vacuum/analyzes,
>> on the ground that foreground operations are usually more important than
>> maintenance tasks.
>
> What this means is that autovacuum will be starved a lot of the time,
> and in the end you will only vacuum the tables when you run out of time
> for Xid wraparound.

Well, only if you do a lot of schema changes. In the previous
discussion, Simon and me agreed that schema changes should not happen on
a regular basis on production systems.

Shouldn't we rather support the regular usage pattern instead of the
uncommon one? Users doing a lot of schema changes are the ones who
should have to work around issues, not those using a DBMS sanely. No?

Best Regards
Michael Paesold


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Michael Paesold <mpaesold(at)gmx(dot)at>, pgsql-hackers(at)postgresql(dot)org, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Re: [COMMITTERS] pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 17:38:43
Message-ID: 19034.1193333923@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Michael Paesold wrote:
>> Yeah, I thought we had agreed that we must cancel all auto vacuum/analyzes,
>> on the ground that foreground operations are usually more important than
>> maintenance tasks.

> What this means is that autovacuum will be starved a lot of the time,

Not really, because DDL changes aren't *that* common (at least not for
non-temp tables). I think the consensus was that we should cancel
autovac in these cases unless it is an anti-wraparound vacuum. Isn't
that why you were putting in the flag to show it is for wraparound?

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Michael Paesold <mpaesold(at)gmx(dot)at>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 17:51:16
Message-ID: 4720D794.1090509@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Michael Paesold wrote:
> In the previous discussion, Simon and me agreed that schema changes
> should not happen on a regular basis on production systems.
>
> Shouldn't we rather support the regular usage pattern instead of the
> uncommon one? Users doing a lot of schema changes are the ones who
> should have to work around issues, not those using a DBMS sanely. No?
>
>

Unfortunately, doing lots of schema changes is a very common phenomenon.
It makes me uncomfortable too, but saying that those who do it have to
work around issues isn't acceptable IMNSHO - it's far too widely done.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Re: [COMMITTERS] pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 19:37:09
Message-ID: 23881.1193341029@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Michael Paesold wrote:
>> Shouldn't we rather support the regular usage pattern instead of the
>> uncommon one? Users doing a lot of schema changes are the ones who
>> should have to work around issues, not those using a DBMS sanely. No?

> Unfortunately, doing lots of schema changes is a very common phenomenon.
> It makes me uncomfortable too, but saying that those who do it have to
> work around issues isn't acceptable IMNSHO - it's far too widely done.

Well, there's going to be pain *somewhere* here, and we already know
that users will find the current 8.3 behavior unacceptable. I'd rather
have autovacuum not make progress than have users turn it off because it
gets in their way too much. Which I think is exactly what will happen
if we ship it with the current behavior.

regards, tom lane


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 19:42:49
Message-ID: 1193341369.4242.322.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Thu, 2007-10-25 at 13:51 -0400, Andrew Dunstan wrote:
>
> Michael Paesold wrote:
> > In the previous discussion, Simon and me agreed that schema changes
> > should not happen on a regular basis on production systems.
> >
> > Shouldn't we rather support the regular usage pattern instead of the
> > uncommon one? Users doing a lot of schema changes are the ones who
> > should have to work around issues, not those using a DBMS sanely. No?
> >
> Unfortunately, doing lots of schema changes is a very common phenomenon.
> It makes me uncomfortable too, but saying that those who do it have to
> work around issues isn't acceptable IMNSHO - it's far too widely done.

We didn't agree that DDL was uncommon, we agreed that running DDL was
more important than running an auto VACUUM. DDL runs very quickly,
unless blocked, though holds up everybody else. So you must run it at
pre-planned windows. VACUUMs can run at any time, so a autoVACUUM
shouldn't be allowed to prevent DDL from running. The queuing DDL makes
other requests queue behind it, even ones that would normally have been
able to execute at same time as the VACUUM.

Anyway, we covered all this before. I started off saying we shouldn't do
this and Heikki and Michael came up with convincing arguments, for me,
so now I think we should allow autovacuums to be cancelled.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Re: [COMMITTERS] pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 19:44:49
Message-ID: 200710251944.l9PJin423381@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > Michael Paesold wrote:
> >> Shouldn't we rather support the regular usage pattern instead of the
> >> uncommon one? Users doing a lot of schema changes are the ones who
> >> should have to work around issues, not those using a DBMS sanely. No?
>
> > Unfortunately, doing lots of schema changes is a very common phenomenon.
> > It makes me uncomfortable too, but saying that those who do it have to
> > work around issues isn't acceptable IMNSHO - it's far too widely done.
>
> Well, there's going to be pain *somewhere* here, and we already know
> that users will find the current 8.3 behavior unacceptable. I'd rather
> have autovacuum not make progress than have users turn it off because it
> gets in their way too much. Which I think is exactly what will happen
> if we ship it with the current behavior.

Agreed. If auto-vacuum is blocking activity, it isn't very 'auto' to
me. If vacuum is blocking something, by definition it is a bad time for
it to be vacuuming and it should abort and try again later.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 19:54:28
Message-ID: 4720F474.9000401@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Simon Riggs wrote:
> On Thu, 2007-10-25 at 13:51 -0400, Andrew Dunstan wrote:
>
>> Michael Paesold wrote:
>>
>>> In the previous discussion, Simon and me agreed that schema changes
>>> should not happen on a regular basis on production systems.
>>>
>>> Shouldn't we rather support the regular usage pattern instead of the
>>> uncommon one? Users doing a lot of schema changes are the ones who
>>> should have to work around issues, not those using a DBMS sanely. No?
>>>
>>>
>> Unfortunately, doing lots of schema changes is a very common phenomenon.
>> It makes me uncomfortable too, but saying that those who do it have to
>> work around issues isn't acceptable IMNSHO - it's far too widely done.
>>
>
> We didn't agree that DDL was uncommon, we agreed that running DDL was
> more important than running an auto VACUUM. DDL runs very quickly,
> unless blocked, though holds up everybody else. So you must run it at
> pre-planned windows. VACUUMs can run at any time, so a autoVACUUM
> shouldn't be allowed to prevent DDL from running. The queuing DDL makes
> other requests queue behind it, even ones that would normally have been
> able to execute at same time as the VACUUM.
>
> Anyway, we covered all this before. I started off saying we shouldn't do
> this and Heikki and Michael came up with convincing arguments, for me,
> so now I think we should allow autovacuums to be cancelled.
>
>

Perhaps I misunderstood, or have been mistunderstood :-) - I am actually
agreeing that autovac should not block DDL.

cheers

andrew


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: pgsql-patches(at)postgresql(dot)org
Subject: Autovacuum cancellation
Date: 2007-10-25 20:07:41
Message-ID: 20071025200741.GJ23566@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Simon Riggs wrote:

> Just noticed you've made these changes. I was working on them, but
> hadn't fully tested the patch because of all the different touch points.
> Sorry for the delay.
>
> Would you like me to refresh my earlier patch against the newly
> committed state (or did you commit that aspect already as well?).

Patch attached, please comment. It only avoids cancelling when the
vacuum is for wraparound.

What we're missing here is doc updates (mainly to lmgr/README, I think)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Attachment Content-Type Size
autovac-cancel.patch text/x-diff 7.0 KB

From: Stefan Kaltenbrunner <stefan(at)kaltenbrunner(dot)cc>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Re: [COMMITTERS] pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 20:28:54
Message-ID: 4720FC86.3030501@kaltenbrunner.cc
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> Michael Paesold wrote:
>>> Shouldn't we rather support the regular usage pattern instead of the
>>> uncommon one? Users doing a lot of schema changes are the ones who
>>> should have to work around issues, not those using a DBMS sanely. No?
>
>> Unfortunately, doing lots of schema changes is a very common phenomenon.
>> It makes me uncomfortable too, but saying that those who do it have to
>> work around issues isn't acceptable IMNSHO - it's far too widely done.
>
> Well, there's going to be pain *somewhere* here, and we already know
> that users will find the current 8.3 behavior unacceptable. I'd rather
> have autovacuum not make progress than have users turn it off because it
> gets in their way too much. Which I think is exactly what will happen
> if we ship it with the current behavior.

exactly - 8.3 will be the first release with autovac enabled by default
(and concurrent autovacuuming) and we really need to make sure that
people wont get surprised by it in the way I (and others) did when
playing with Beta1.
So my vote would be on cancelling always except in the case of a
wraparound vacuum.

Stefan


From: David Fetter <david(at)fetter(dot)org>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Michael Paesold <mpaesold(at)gmx(dot)at>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-committers(at)postgresql(dot)org, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: pgsql: Extract catalog info for error reporting before an error actually
Date: 2007-10-25 21:04:58
Message-ID: 20071025210458.GA17890@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Thu, Oct 25, 2007 at 03:54:28PM -0400, Andrew Dunstan wrote:
> Simon Riggs wrote:
> >On Thu, 2007-10-25 at 13:51 -0400, Andrew Dunstan wrote:
> >>Michael Paesold wrote:
> >>
> >>>In the previous discussion, Simon and me agreed that schema
> >>>changes should not happen on a regular basis on production
> >>>systems.
> >>>
> >>>Shouldn't we rather support the regular usage pattern instead of
> >>>the uncommon one? Users doing a lot of schema changes are the
> >>>ones who should have to work around issues, not those using a
> >>>DBMS sanely. No?
> >>>
> >>Unfortunately, doing lots of schema changes is a very common
> >>phenomenon. It makes me uncomfortable too, but saying that those
> >>who do it have to work around issues isn't acceptable IMNSHO -
> >>it's far too widely done.
> >
> >We didn't agree that DDL was uncommon, we agreed that running DDL
> >was more important than running an auto VACUUM. DDL runs very
> >quickly, unless blocked, though holds up everybody else. So you
> >must run it at pre-planned windows. VACUUMs can run at any time, so
> >a autoVACUUM shouldn't be allowed to prevent DDL from running. The
> >queuing DDL makes other requests queue behind it, even ones that
> >would normally have been able to execute at same time as the
> >VACUUM.
> >
> >Anyway, we covered all this before. I started off saying we
> >shouldn't do this and Heikki and Michael came up with convincing
> >arguments, for me, so now I think we should allow autovacuums to be
> >cancelled.
>
> Perhaps I misunderstood, or have been mistunderstood :-) - I am
> actually agreeing that autovac should not block DDL.

+1 here for having autovacuum not block DDL :)

Cheers,
David (for what it's worth)
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-25 21:35:46
Message-ID: 26532.1193348146@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Patch attached, please comment. It only avoids cancelling when the
> vacuum is for wraparound.

I'm not entirely convinced that there can be only one autovac proc in
the portion of the waits-for graph explored by DeadlockCheck. If there
is more than one, we'll cancel a random one of them, which seems OK ---
but the comment added to FindLockCycleRecurse is bogus.

I thought about suggesting that we test PROC_VACUUM_FOR_WRAPAROUND
before setting blocking_autovacuum_proc at all, but I guess the reason
you don't do that is you don't want to take ProcArrayLock there (and we
decided it was unsafe to check the bit without the lock). So the other
thing that comment block needs is a note that it seems OK to check
PROC_IS_AUTOVACUUM without the lock, because it never changes for an
existing PGPROC, but not PROC_VACUUM_FOR_WRAPAROUND.

Otherwise it looks OK --- a bit ugly but I don't have a better idea.

There's some things still to be desired here: if an autovac process is
involved in a hard deadlock, the patch doesn't favor zapping it over
anybody else, nor consider cancelling the autovac as an alternative to
rearranging queues for a soft deadlock. But dealing with that will open
cans of worms that I don't think we want to open for 8.3.

regards, tom lane


From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Autovacuum cancellation
Date: 2007-10-25 22:28:39
Message-ID: 87abq696co.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> There's some things still to be desired here: if an autovac process is
> involved in a hard deadlock, the patch doesn't favor zapping it over
> anybody else, nor consider cancelling the autovac as an alternative to
> rearranging queues for a soft deadlock. But dealing with that will open
> cans of worms that I don't think we want to open for 8.3.

Can autovacuum actually get into a hard deadlock? Does it take more than one
lock that can block others at the same time?

I think there's a window where the process waiting directly on autovacuum
could have already fired its deadlock check before it was waiting directly on
autovacuum. But the only way I can see it happening is if another process is
cancelled before its deadlock check fires and the signals are processed out of
order. I'm not sure that's a case we really need to worry about.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Gregory Stark <stark(at)enterprisedb(dot)com>
Cc: "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 00:38:42
Message-ID: 29572.1193359122@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> Can autovacuum actually get into a hard deadlock?

It can certainly be part of a deadlock loop, though the practical cases
might be few. It will be holding more than one lock, eg a lock on its
target table and various transient locks on system catalogs, and other
processes taking or trying for exclusive locks on those things could
create issues. I think it's OK to leave the issue go for now, until
we see if anyone hits it in practice --- I just wanted to mention that
we might need to consider the problem in future.

> I think there's a window where the process waiting directly on
> autovacuum could have already fired its deadlock check before it was
> waiting directly on autovacuum.

I think you don't understand what that code is doing. If there's an
autovac anywhere in the dependency graph, it'll find it.

regards, tom lane


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Gregory Stark <stark(at)enterprisedb(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 01:11:02
Message-ID: 20071026011102.GO23566@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Tom Lane wrote:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:

> > I think there's a window where the process waiting directly on
> > autovacuum could have already fired its deadlock check before it was
> > waiting directly on autovacuum.
>
> I think you don't understand what that code is doing. If there's an
> autovac anywhere in the dependency graph, it'll find it.

Thanks for making that explicit, I was trying to build a test case where
it would lock :-)

(If we had concurrent psql we could even have it in the regression tests
...)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


From: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 09:32:09
Message-ID: 4721B419.1070700@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Alvaro Herrera wrote:
> /*
> * Look for a blocking autovacuum. There will only ever
> * be one, since the autovacuum workers are careful
> * not to operate concurrently on the same table.
> */

I think that's a bit unaccurate. You could have multiple autovacuum
workers operating on different tables participating in a deadlock. The
reason that can't happen is that autovacuum never holds a lock while
waiting for another.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 09:49:21
Message-ID: 87ve8uxl26.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches


"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

>> I think there's a window where the process waiting directly on
>> autovacuum could have already fired its deadlock check before it was
>> waiting directly on autovacuum.
>
> I think you don't understand what that code is doing. If there's an
> autovac anywhere in the dependency graph, it'll find it.

That'll teach me to try to read code from a patch directly without trying to
apply it or at least read the original source next to it. I thought I had seen
this code recently enough to apply the patch from memory -- clearly not.

I assume the right thing happens if multiple deadlock check signals fire for
the same autovacuum?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 12:56:37
Message-ID: 10572.1193403397@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> Alvaro Herrera wrote:
>> /*
>> * Look for a blocking autovacuum. There will only ever
>> * be one, since the autovacuum workers are careful
>> * not to operate concurrently on the same table.
>> */

> I think that's a bit unaccurate. You could have multiple autovacuum
> workers operating on different tables participating in a deadlock. The
> reason that can't happen is that autovacuum never holds a lock while
> waiting for another.

And that's not true either. It may only want low-grade locks (eg
AccessShareLock on a system catalog) but deadlock is nonetheless
entirely possible in principle.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Gregory Stark <stark(at)enterprisedb(dot)com>
Cc: "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 12:59:47
Message-ID: 10623.1193403587@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> I assume the right thing happens if multiple deadlock check signals fire for
> the same autovacuum?

Multiple signals shouldn't be a problem, but late-arriving ones could be.
It might be worth having autovac explicitly clear QueryCancelPending
after it's finished a table, so that a SIGINT sent because of activity
on one table couldn't force cancellation of vacuum on the next one.

regards, tom lane


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Gregory Stark <stark(at)enterprisedb(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-26 20:50:31
Message-ID: 20071026205031.GD6725@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

Tom Lane wrote:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> > I assume the right thing happens if multiple deadlock check signals fire for
> > the same autovacuum?
>
> Multiple signals shouldn't be a problem, but late-arriving ones could be.
> It might be worth having autovac explicitly clear QueryCancelPending
> after it's finished a table, so that a SIGINT sent because of activity
> on one table couldn't force cancellation of vacuum on the next one.

Ok, committed; I snuck that in as well, but I'm not sure how to test
that it works.

I adjusted the comments -- I think they're more correct now. I also
added a puny paragraph to lmgr/README.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-27 22:15:12
Message-ID: 1193523312.4242.623.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Thu, 2007-10-25 at 17:35 -0400, Tom Lane wrote:
> There's some things still to be desired here: if an autovac process is
> involved in a hard deadlock, the patch doesn't favor zapping it over
> anybody else, nor consider cancelling the autovac as an alternative to
> rearranging queues for a soft deadlock. But dealing with that will
> open cans of worms that I don't think we want to open for 8.3.

I did look at doing that but decided it would not be appropriate to do
that in all cases. i.e. there are hard deadlock cases where the autovac
can be the head of the lock queue and yet a deadlock still exists
between two other processes. The deadlock detector doesn't get called
twice for the same deadlock, so it wasn't possible to speculatively do
that and then re-catch it second time around.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-27 22:22:40
Message-ID: 1193523760.4242.628.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Fri, 2007-10-26 at 10:32 +0100, Heikki Linnakangas wrote:
> Alvaro Herrera wrote:
> > /*
> > * Look for a blocking autovacuum. There will only ever
> > * be one, since the autovacuum workers are careful
> > * not to operate concurrently on the same table.
> > */
>
> I think that's a bit unaccurate. You could have multiple autovacuum
> workers operating on different tables participating in a deadlock. The
> reason that can't happen is that autovacuum never holds a lock while
> waiting for another.

I wrote that code comment; as you say it is true only when there are at
least 4 processes in the lock graph where 2+ normal backends are
deadlocking and there are 2+ autovacuums holding existing locks. The
comment should have said "If blocking is caused by an autovacuum process
then ... (there will)".

The blocking_autovacuum_proc doesn't react unless there are no hard
deadlocks, so the code works.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Gregory Stark <stark(at)enterprisedb(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-27 22:31:02
Message-ID: 1193524262.4242.636.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Fri, 2007-10-26 at 17:50 -0300, Alvaro Herrera wrote:

> Ok, committed; I snuck that in as well, but I'm not sure how to test
> that it works.

I've had time to review that now. I didn't reply to your original post
because you'd taken my name off the copy list for some reason and I've
been too busy to read non-addressed mail.

The committed patch is pretty much the same as my original AFAICS.

I'm sure you didn't mean to forget that, but can you please acknowledge
my contribution in CVS? Thanks.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Autovacuum cancellation
Date: 2007-10-28 08:46:32
Message-ID: 1193561192.4242.649.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers pgsql-patches

On Sat, 2007-10-27 at 23:22 +0100, Simon Riggs wrote:
> On Fri, 2007-10-26 at 10:32 +0100, Heikki Linnakangas wrote:
> > Alvaro Herrera wrote:
> > > /*
> > > * Look for a blocking autovacuum. There will only ever
> > > * be one, since the autovacuum workers are careful
> > > * not to operate concurrently on the same table.
> > > */
> >
> > I think that's a bit unaccurate. You could have multiple autovacuum
> > workers operating on different tables participating in a deadlock. The
> > reason that can't happen is that autovacuum never holds a lock while
> > waiting for another.
>
> I wrote that code comment; as you say it is true only when there are at
> least 4 processes in the lock graph where 2+ normal backends are
> deadlocking and there are 2+ autovacuums holding existing locks. The
> comment should have said "If blocking is caused by an autovacuum process
> then ... (there will)".

Sorry...this should read "as you say it is **not** true".

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com