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
