Bug? Concurrent COMMENT ON and DROP object
in [PgSql]
|
Prev: I: [HACKERS] About "Our CLUSTER implementation is pessimal" patch
Next: pgsql: Fix log_temp_files docs and comments to say bytes not kilobytes.
From: Robert Haas on 6 Jul 2010 09:45 2010/7/6 KaiGai Kohei <kaigai(a)ak.jp.nec.com>: > In the following scenario, we can see orphan comments. Yeah. I think the reason we haven't seen any complaints about this before is that the worst-case scenario is that a comment for a dropped database object eventually becomes associated with a new database object. But that can only happen if the OID counter wraps around and then OID then gets reused for another object of the same type, and even then you might easily fail to notice. Still, it would be nice to clean this up. > It says the purpose of the relation_openrv() to �acquire a lock that > ensures no one else drops the relation before we commit. So, I was > blocked when I tried to comment on the table which was already dropped > in another session but uncommited yet. > However, it is not a problem limited to relations. For example, we need > to acquire a lock on the pg_type catalog using > > For example, we need to acquire a lock on the pg_type catalog when we > try to comment on any type object. Perhaps, I think LockRelationOid() > should be injected at head of the CommentType() in this case. > > Any comments? A more fine-grained lock would be preferable, if we can manage it. Can we just lock the relevant pg_type tuple, rather than the whole relation? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 6 Jul 2010 10:33 Robert Haas <robertmhaas(a)gmail.com> writes: > 2010/7/6 KaiGai Kohei <kaigai(a)ak.jp.nec.com>: >> In the following scenario, we can see orphan comments. > Yeah. I think the reason we haven't seen any complaints about this > before is that the worst-case scenario is that a comment for a dropped > database object eventually becomes associated with a new database > object. Well, in general there is very little DDL locking for any object type other than tables. I think the original rationale for that was that most other object types are defined by single catalog entries, so that attempts to update/delete the object would naturally block on changing its tuple anyway. But between comments and pg_depend entries that seems not particularly true anymore. IIRC there is now some attempt to lock objects of all types during DROP. Maybe the COMMENT code could acquire a conflicting lock. >> For example, we need to acquire a lock on the pg_type catalog when we >> try to comment on any type object. Perhaps, I think LockRelationOid() >> should be injected at head of the CommentType() in this case. >> >> Any comments? > A more fine-grained lock would be preferable, s/preferable/essential/. This cure would be *far* worse than the disease. Can you say "deadlock"? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 6 Jul 2010 19:46 (2010/07/06 23:33), Tom Lane wrote: > Robert Haas<robertmhaas(a)gmail.com> writes: >> 2010/7/6 KaiGai Kohei<kaigai(a)ak.jp.nec.com>: >>> In the following scenario, we can see orphan comments. > >> Yeah. I think the reason we haven't seen any complaints about this >> before is that the worst-case scenario is that a comment for a dropped >> database object eventually becomes associated with a new database >> object. > > Well, in general there is very little DDL locking for any object type > other than tables. I think the original rationale for that was that > most other object types are defined by single catalog entries, so that > attempts to update/delete the object would naturally block on changing > its tuple anyway. But between comments and pg_depend entries that seems > not particularly true anymore. > > IIRC there is now some attempt to lock objects of all types during > DROP. Maybe the COMMENT code could acquire a conflicting lock. > Are you saying AcquireDeletionLock()? It seems to me fair enough to prevent the problem, although it is declared as a static function. >>> For example, we need to acquire a lock on the pg_type catalog when we >>> try to comment on any type object. Perhaps, I think LockRelationOid() >>> should be injected at head of the CommentType() in this case. >>> >>> Any comments? > >> A more fine-grained lock would be preferable, > > s/preferable/essential/. This cure would be *far* worse than the > disease. Can you say "deadlock"? > > regards, tom lane > -- KaiGai Kohei <kaigai(a)ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 6 Jul 2010 20:16 2010/7/6 KaiGai Kohei <kaigai(a)ak.jp.nec.com>: > (2010/07/06 23:33), Tom Lane wrote: >> Robert Haas<robertmhaas(a)gmail.com> �writes: >>> 2010/7/6 KaiGai Kohei<kaigai(a)ak.jp.nec.com>: >>>> In the following scenario, we can see orphan comments. >> >>> Yeah. �I think the reason we haven't seen any complaints about this >>> before is that the worst-case scenario is that a comment for a dropped >>> database object eventually becomes associated with a new database >>> object. >> >> Well, in general there is very little DDL locking for any object type >> other than tables. �I think the original rationale for that was that >> most other object types are defined by single catalog entries, so that >> attempts to update/delete the object would naturally block on changing >> its tuple anyway. �But between comments and pg_depend entries that seems >> not particularly true anymore. >> >> IIRC there is now some attempt to lock objects of all types during >> DROP. �Maybe the COMMENT code could acquire a conflicting lock. >> > Are you saying AcquireDeletionLock()? > > It seems to me fair enough to prevent the problem, although it is declared > as a static function. Obviously not. We don't need to acquire an AccessExclusiveLock to comment on an object - just something that will CONFLICT WITH an AccessExclusiveLock. So, use the same locking rules, perhaps, but take a much weaker lock, like AccessShareLock. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 6 Jul 2010 22:18
Robert Haas <robertmhaas(a)gmail.com> writes: > Obviously not. We don't need to acquire an AccessExclusiveLock to > comment on an object - just something that will CONFLICT WITH an > AccessExclusiveLock. So, use the same locking rules, perhaps, but > take a much weaker lock, like AccessShareLock. Well, it probably needs to be a self-conflicting lock type, so that two COMMENTs on the same object can't run concurrently. But I agree AccessExclusiveLock is too strong: that implies locking out read-only examination of the object, which we don't want. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |