Re: We need to log aborted autovacuums - Mailing list pgsql-hackers

From Greg Smith
Subject Re: We need to log aborted autovacuums
Date
Msg-id 4D331B74.6000004@2ndquadrant.com
Whole thread Raw
In response to Re: We need to log aborted autovacuums  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: We need to log aborted autovacuums  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Robert Haas wrote: <blockquote cite="mid:AANLkTi=t_ay3Rq3LQW=N8VH9Tuj_npSufe88zLLChaaY@mail.gmail.com"
type="cite"><prewrap="">On Sat, Jan 15, 2011 at 11:14 AM, Tom Lane <a class="moz-txt-link-rfc2396E"
href="mailto:tgl@sss.pgh.pa.us"><tgl@sss.pgh.pa.us></a>wrote: </pre><blockquote type="cite"><pre wrap="">Greg
Smith<a class="moz-txt-link-rfc2396E" href="mailto:greg@2ndquadrant.com"><greg@2ndquadrant.com></a> writes:
</pre><blockquotetype="cite"><pre wrap="">Does try_relation_open need to have a lock acquisition timeout when AV
 
is calling it?     </pre></blockquote><pre wrap="">Hmm.  I think when looking at the AV code, I've always
subconsciously
assumed that try_relation_open would fail immediately if it couldn't get
the lock.  That certainly seems like it would be a more appropriate way
to behave than delaying indefinitely.   </pre></blockquote><pre wrap="">
I'm confused how that's not happening already. What does "try" mean, otherwise? </pre></blockquote><br /> Apparently
"try"means acquire the requested lock on the oid of the relation, waiting for any amount of time for that part, and
thencheck if the relation really exists or not once it's got it.  In this context, it means it will try to open the
relation,but might fail if it doesn't actually exist anymore.  The relation not existing once it tries the check done
afterthe lock is acquired is the only way it will return a failure.<br /><br /> try_relation_open calls
LockRelationOid,which blocks.  There is also a ConditionalLockRelationOid, which does the same basic thing except it
exitsimmediately, with a false return code, if it can't acquire the lock.  I think we just need to nail down in what
existingcases it's acceptable to have try_relation_oid use ConditionalLockRelationOid instead, which would make it do
whatall us reading the code thought it did all along.<br /><br /> There are four callers of try_relation_open to be
concernedabout here:<br /><br /> src/backend/commands/vacuum.c    vacuum_rel<br />     onerel =
try_relation_open(relid,lmode);<br /><br /> src/backend/commands/analyze.c    analyze_rel<br />     onerel =
try_relation_open(relid,ShareUpdateExclusiveLock);<br /><br /> src/backend/commands/cluster.c    cluster_rel<br />    
OldHeap= try_relation_open(tableOid, AccessExclusiveLock);<br /><br /> src/backend/commands/lockcmds.c
LockTableRecurse<br/>     * Apply LOCK TABLE recursively over an inheritance tree<br />     rel =
try_relation_open(reloid,NoLock);<br /><br /> I think that both the vacuum_rel and analyze_rel cases are capable of
figuringout if they are the autovacuum process, and if so calling the fast non-blocking version of this.  I wouldn't
wantto mess with the other two, which rely upon the current behavior as far as I can see.<br /><br /> Probably took me
longerto write this e-mail than the patch will take.  Since I've got trivial patch fever this weekend and already have
thetest case, I'll do this one next.<br /><br /><pre class="moz-signature" cols="72">-- 
 
Greg Smith   2ndQuadrant US    <a class="moz-txt-link-abbreviated"
href="mailto:greg@2ndQuadrant.com">greg@2ndQuadrant.com</a>  Baltimore, MD
 
PostgreSQL Training, Services, and 24x7 Support  <a class="moz-txt-link-abbreviated"
href="http://www.2ndQuadrant.us">www.2ndQuadrant.us</a>
"PostgreSQL 9.0 High Performance": <a class="moz-txt-link-freetext"
href="http://www.2ndQuadrant.com/books">http://www.2ndQuadrant.com/books</a>
</pre>

pgsql-hackers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: Recovery control functions
Next
From: Tom Lane
Date:
Subject: Re: replication and pg_hba.conf