Re: [GENERAL] Using an SMP machine to make multiple indices on

Tom Lane wrote:
> 
> "Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> > In the end, I changed DefineIndex() to not call IndexesAreActive().
> 
> I saw that.  But is it a good solution?  If someone has deactivated
> indexes on a user table (ie turned off relhasindex), then creating a
> new index would activate them again, which would probably be bad.

I apolgize my neglect of reconsidering the activte/deactivate
stuff for indexes. Probably it is no longer needed now(since 7.1).
Reindex under postmaster for user tables has been available 
from the first. I didn't write a documentation about it inten-
tionally in 7.0 though it was my neglect also in 7.1 sorry.
In 7.0 REINDEX set relhasindex to false first to tell all 
backends that the indexes are unavailable because we wasn't 
able to recreate indexes safely in case of abort. Note
that relhasindex was set immediately(out of transactional
control) in 7.0 and acruiring a lock for the pg_class tuple
was very critical.
Since 7.1 we are able to recreate indexes safely under 
postmaster and REINDEX doesn't set relhasindex to false
for user tables. Though REINDEX deactivates the indexes of
system tables the deactivation is done under transactional
control and other backends never see the deactivated 
relhasindex.

> 
> I have realized that this code is wrong anyway, because it doesn't
> acquire ShareLock on the relation until far too late; all the setup
> processing is done with no lock at all :-(.  LockClassinfoForUpdate
> provided a little bit of security against concurrent schema changes,
> though not enough.
> 
> Also, I'm now a little worried about whether concurrent index creations
> will actually work.  Both CREATE INDEX operations will try to update
> the pg_class tuple to set relhasindex true.

Yes but there's a big difference. It's at the end of the creation
not at the beginning. Also note that UpdateStats() updates pg_class
tuple in case of B-trees etc before updating relhasindex. I'm
suspicios if we should update Stats under the transactional control. 
 Since they use
> simple_heap_update for that, the second one is likely to fail
> because simple_heap_update doesn't handle concurrent updates.
> 
> I think what we probably want is
> 
>         1. Acquire ShareLock at the very start.
> 
>         2. Check for indexes present but relhasindex = false,
>            if so complain.
> 
>         3. Build the index.
> 
>         4. Update pg_class tuple, being prepared for concurrent
>            updates (ie, do NOT use simple_heap_update here).
> 
> I still don't see any value in LockClassinfoForUpdate, however.

ISTM to rely on completely the lock for the corresponding
relation is a little misplaced. For example ALTER TABLE OWNER
doesn't acquire any lock on the table but it seems natural to me.
UPDATE pg_class set .. doesn't acquire any lock on the correspoding
relations of the target pg_class tuples but it seems natural to me,

regards,
Hiroshi Inoue