pgsql: Extract catalog info for error reporting before an error actually
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)
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
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)
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
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
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"
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
Alvaro Herrera <alvherre@commandprompt.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
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
Andrew Dunstan <andrew@dunslane.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
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
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.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@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
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
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.
Attachments:
autovac-cancel.patchtext/x-diff; charset=us-asciiDownload+91-5
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.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
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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Alvaro Herrera <alvherre@commandprompt.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
"Tom Lane" <tgl@sss.pgh.pa.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
Gregory Stark <stark@enterprisedb.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
Tom Lane wrote:
Gregory Stark <stark@enterprisedb.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.
