On 27.03.2013 14:50, Robert Haas wrote:
On Sat, Mar 23, 2013 at 6:45 PM, Xi Wang<xi.w...@gmail.com> wrote:
A side question: at src/backend/storage/lmgr/proc.c:1150, is there a
null pointer deference for `autovac'?
I think you mean on line 1142:
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
*HERE* PGXACT *autovac_pgxact = &ProcGlobal->allPgXact[autovac->pgprocno];
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
/*
* Only do it if the worker is not working to protect against
Xid
* wraparound.
*/
if ((autovac != NULL) &&
(autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) &&
!(autovac_pgxact->vacuumFlags &
PROC_VACUUM_FOR_WRAPAROUND))
Not really. If the deadlock_state is DS_BLOCKED_BY_AUTOVACUUM, there
has to be a blocking autovacuum proc. As in the other case that you
found, though, some code rearrangement would likely make the intent of
the code more clear and avoid future mistakes.
Perhaps something like:
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM&&
allow_autovacuum_cancel
&& (autovac = GetBlockingAutoVacuumPgproc()) != NULL)
{
PGXACT *autovac_pgxact =
&ProcGlobal->allPgXact[autovac->pgprocno];
...
Writing it like that suggests that autovac might sometimes be NULL, even
if deadlock_state == DS_BLOCKED_BY_AUTOVACUUM. From your explanation
above, I gather that's not possible (and I think you're right), so the
NULL check is unnecessary. If we think it might be NULL after all, the
above makes sense.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
