Request for review,

[Christopher Powers]

Howdy,

I finally got this changeset off of my local system and into pushbuild. 
Please don't be afraid--these are not huge changes.

http://lists.mysql.com/commits/63274

Kevin or Vlad, you would be good choices to review the Storage Interface 
changes since you've both been in and out of that code. John or Hakan, 
maybe you could take a quick look at the testcase changes.

Changes of note:

1) Online add/drop of primary keys occur via the internal 'upgrade' 
process rather than a direct call to createIndex(). Primary keys are a 
different animal than ordinary keys, so rather than enhance the existing 
online ADD INDEX, I simply used the mechanism already in place.

2) Improved mapping of Falcon and server indexes in StorageTable and 
StorageTableShare. Sometimes the mapping could get out-of-sync and the 
index ids for a table would be different between the server and Falcon.

3) During stress testing, I found that a concurrent SELECT or other 
operation can sneak in between online ALTER phases 2 and 3. This is a 
problem for DROP INDEX because at phase 2, Falcon has deleted the index 
but the server has not.

This manifests as a fatal assertion in the server following a call to 
StorageInterface::init_index(), which fails if the index has been 
deleted. The problem is that the server does not provide DDL protection 
across the three phases of online ALTER (see 
StorageInterface::alter_table_phase1, 2 and 3).

As a workaround, Falcon does a quick check 
(StorageTable::checkCurrentIndex) prior to each index operation 
(StorageInterface::index_scan, index_next, etc) rather than only in 
index_init(), and returns a more benign error (HA_ERR_KEY_NOT_FOUND) if 
the server tries to access a deleted index. This allows for a more 
graceful failure, however, we should never have to contend with it in 
the first place--the server should know better than to access an index 
it is in the process of deleting.

I've discussed this with Kostja and Martin Skold. Both agree it is a 
problem, bug TBD.

4) The CREATE INDEX operation, like all DDL, gets an exclusive lock on 
syncSysDDL. The Scavenger always gets a shared lock on syncSysDDL. This 
is a problem because CREATE INDEX populates the index before releasing 
the lock. Without scavenging, this will blow out the record cache for 
large tables, so I am now working on a method to decouple syncSysDDL and 
the Scavenger. See Bug#35755, "Cannot create index on Falcon table...".



Thanks,

Chris




-