Gregory Stark <[EMAIL PROTECTED]> writes: > A few concerns > a) The use of ShareUpdateExclusiveLock is supposed to lock out concurrent > vacuums. I just tried it and vacuum seemed to be unaffected.
Can't reproduce such a problem here. > Do we still need to block concurrent vacuums if we're using snapshots? That's an interesting question. I don't think we need to lock out vacuum in either pass as far as tuple removal goes: we are only interested in tuples that are visible to a live transaction (ie ours) and vacuum shouldn't remove them. However one of the properties of ShareUpdateExclusive is supposed to be that it locks out schema changes ... such as creation of a new index. I wouldn't want to bet that vacuum works correctly if a new index appears partway through its collection of info about the relation. I think we need to keep that lock so that vacuum is safe against create index, rather than vice versa. > b) You introduced a LockRelationIdForSession() call (I even didn't realize we > had this capability when I wrote the patch). Does this introduce the > possibility of a deadlock though? Indeed, I had noticed this while testing your point (a). I thought that ConditionalLockRelation had enough smarts to grant itself a lock if there would be a deadlock possibility otherwise, but it seems not to be noticing. We'll need to look into that. > c) It's a shame we don't support multiple concurrent concurrent index builds. I can't get excited about that. The point of this facility is to build indexes with minimal impact on production queries, and the I/O load imposed by even one build is going to make that a bit questionable, let alone multiple ones. If you want to do some useful improvement on the patch, look into making it throttle its I/O rate like vacuum can. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster