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
|
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: