Re: [HACKERS] Dead code or buggy code?
On Fri, Sep 20, 2013 at 12:13:18AM +0100, Greg Stark wrote:
So I'm just going to make the code defensive and assume NULL is possible when
if maybe it isn't.
In case it's not clear, this is one of the thing's Xi Wang's took picked up.
There not to many but it turns out they are indeed not all in the adt code so
I
might wait until after the commit fest to commit it to avoid causing bit
churn.
Uh, where are we on this?
---
--
greg
On 19 Sep 2013 12:52, Robert Haas robertmh...@gmail.com wrote:
On Wed, Sep 18, 2013 at 6:20 PM, Greg Stark st...@mit.edu wrote:
The following code is in the ProcSleep at proc.c:1138.
GetBlockingAutoVacuumPgproc() should presumably always return a vacuum
pgproc entry since the deadlock state says it's blocked by autovacuum.
But I'm not really familiar enough with this codepath to know whether
there's not a race condition here where it can sometimes return null.
The following code checks autovac != NULL but the PGXACT initializer
would have seg faulted if it returned NULL if that's possible.
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel)
{
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
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))
{
Hmm, yeah. I remember noticing this some time ago but never got
around to fixing it. +1 for rearranging things there somehow.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dead code or buggy code?
On Wed, Sep 18, 2013 at 6:20 PM, Greg Stark st...@mit.edu wrote:
The following code is in the ProcSleep at proc.c:1138.
GetBlockingAutoVacuumPgproc() should presumably always return a vacuum
pgproc entry since the deadlock state says it's blocked by autovacuum.
But I'm not really familiar enough with this codepath to know whether
there's not a race condition here where it can sometimes return null.
The following code checks autovac != NULL but the PGXACT initializer
would have seg faulted if it returned NULL if that's possible.
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel)
{
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
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))
{
Hmm, yeah. I remember noticing this some time ago but never got
around to fixing it. +1 for rearranging things there somehow.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dead code or buggy code?
So I'm just going to make the code defensive and assume NULL is possible
when if maybe it isn't.
In case it's not clear, this is one of the thing's Xi Wang's took picked
up. There not to many but it turns out they are indeed not all in the adt
code so I might wait until after the commit fest to commit it to avoid
causing bit churn.
--
greg
On 19 Sep 2013 12:52, Robert Haas robertmh...@gmail.com wrote:
On Wed, Sep 18, 2013 at 6:20 PM, Greg Stark st...@mit.edu wrote:
The following code is in the ProcSleep at proc.c:1138.
GetBlockingAutoVacuumPgproc() should presumably always return a vacuum
pgproc entry since the deadlock state says it's blocked by autovacuum.
But I'm not really familiar enough with this codepath to know whether
there's not a race condition here where it can sometimes return null.
The following code checks autovac != NULL but the PGXACT initializer
would have seg faulted if it returned NULL if that's possible.
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel)
{
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
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))
{
Hmm, yeah. I remember noticing this some time ago but never got
around to fixing it. +1 for rearranging things there somehow.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
[HACKERS] Dead code or buggy code?
The following code is in the ProcSleep at proc.c:1138.
GetBlockingAutoVacuumPgproc() should presumably always return a vacuum
pgproc entry since the deadlock state says it's blocked by autovacuum.
But I'm not really familiar enough with this codepath to know whether
there's not a race condition here where it can sometimes return null.
The following code checks autovac != NULL but the PGXACT initializer
would have seg faulted if it returned NULL if that's possible.
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel)
{
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
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))
{
--
greg
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
