From: Date: August 17 2008 12:04am Subject: bzr commit into mysql-6.0-falcon branch (cpowers:2788) Bug#38044 List-Archive: http://lists.mysql.com/commits/51810 X-Bug: 38044 Message-Id: <20080816220440.15AAF1DB072A@xeno.mysql.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///home/cpowers/work/dev/dev-04a/mysql/ 2788 Christopher Powers 2008-08-16 Bug#38044 Falcon crash in StorageTable::compareKey Removed debugging code Added StorageTableShare::syncIndexes modified: storage/falcon/Index.cpp storage/falcon/Index.h storage/falcon/StorageTable.cpp storage/falcon/StorageTable.h storage/falcon/StorageTableShare.cpp storage/falcon/StorageTableShare.h storage/falcon/Table.cpp storage/falcon/ha_falcon.cpp per-file messages: storage/falcon/Index.cpp Removed debug code storage/falcon/Index.h Removed debug code storage/falcon/StorageTable.cpp Removed debug code Renamed haveShareLock to indexesLocked storage/falcon/StorageTable.h Removed debug code Renamed haveShareLock to indexesLocked storage/falcon/StorageTableShare.cpp Removed debug code Added syncIndexes storage/falcon/StorageTableShare.h Removed debug code Added syncIndexes storage/falcon/Table.cpp Added comments storage/falcon/ha_falcon.cpp Removed debug code Lock StorageTableShare::syncIndexes for index operations === modified file 'storage/falcon/Index.cpp' --- a/storage/falcon/Index.cpp 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/Index.cpp 2008-08-16 22:04:29 +0000 @@ -85,7 +85,7 @@ Index::Index(Table * tbl, const char * i void Index::init(Table *tbl, const char *indexName, int indexType, int count) { - useCount = 1; + // useCount = 1; // debug table = tbl; database = table->database; dbb = table->dbb; @@ -117,7 +117,7 @@ void Index::init(Table *tbl, const char Index::~Index() { - ASSERT(useCount <= 2); + // ASSERT(useCount <= 2); // debug if (deferredIndexes.first) { @@ -1144,6 +1144,7 @@ void Index::scanDIHash(IndexKey* scanKey } } +#if 0 //debug void Index::lock(bool exclusiveLock) { syncObject.lock(NULL, (exclusiveLock) ? Exclusive : Shared); @@ -1170,7 +1171,7 @@ void Index::release() Table *t = table; const char *tableName = table->name; ***/ - //delete this; //cwp disabled for debug + //delete this; // disabled for debug } } - +#endif === modified file 'storage/falcon/Index.h' --- a/storage/falcon/Index.h 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/Index.h 2008-08-16 22:04:29 +0000 @@ -129,10 +129,13 @@ public: Index(Table *tbl, const char *indexName, int indexType, int id, int numberFields); virtual ~Index(); + /*** debug virtual void lock(bool exclusiveLock); virtual void unlock(void); void release(); void addRef(); + volatile INTERLOCK_TYPE useCount; + ***/ Table *table; Database *database; @@ -162,7 +165,6 @@ public: SyncObject syncDIHash; SyncObject syncUnique; IndexWalker* positionIndex(IndexKey* lowKey, IndexKey* highKey, int searchFlags, Transaction* transaction); - volatile INTERLOCK_TYPE useCount; }; #endif // !defined(AFX_INDEX_H__02AD6A44_A433_11D2_AB5B_0000C01D2301__INCLUDED_) === modified file 'storage/falcon/StorageTable.cpp' --- a/storage/falcon/StorageTable.cpp 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/StorageTable.cpp 2008-08-16 22:04:29 +0000 @@ -51,7 +51,7 @@ StorageTable::StorageTable(StorageConnec upperBound = lowerBound = NULL; record = NULL; recordLocked = false; - haveShareLock = false; + indexesLocked = false; } StorageTable::~StorageTable(void) @@ -100,8 +100,6 @@ int StorageTable::truncateTable(void) return ret; } - - int StorageTable::insert(void) { try @@ -188,10 +186,10 @@ void StorageTable::transactionEnded(void int StorageTable::setCurrentIndex(int indexId) { - if (!haveShareLock) + if (!indexesLocked) { - share->lock(); - haveShareLock = true; + share->lockIndexes(); + indexesLocked = true; } if (!(currentIndex = share->getIndex(indexId))) @@ -200,10 +198,12 @@ int StorageTable::setCurrentIndex(int in return StorageErrorNoIndex; } + /*** debug if (currentIndex->index) - currentIndex->index->addRef(); //cwp debug + currentIndex->index->addRef(); else currentIndex->index = NULL; + ***/ upperBound = lowerBound = NULL; searchFlags = 0; @@ -212,16 +212,18 @@ int StorageTable::setCurrentIndex(int in int StorageTable::clearCurrentIndex() { + /*** debug if (currentIndex) - if (currentIndex->index) //cwp tbd + if (currentIndex->index) currentIndex->index->release(); else - currentIndex->index = NULL; // cwp debug + currentIndex->index = NULL; + ***/ - if (haveShareLock) + if (indexesLocked) { - share->unlock(); - haveShareLock = false; + share->unlockIndexes(); + indexesLocked = false; } currentIndex = NULL; @@ -256,7 +258,6 @@ int StorageTable::indexScan(int indexOrd return 0; } - void StorageTable::clearBitmap(void) { if (bitmap) === modified file 'storage/falcon/StorageTable.h' --- a/storage/falcon/StorageTable.h 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/StorageTable.h 2008-08-16 22:04:29 +0000 @@ -128,7 +128,7 @@ public: Stream insertStream; int searchFlags; bool recordLocked; - bool haveShareLock; + bool indexesLocked; }; #endif === modified file 'storage/falcon/StorageTableShare.cpp' --- a/storage/falcon/StorageTableShare.cpp 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/StorageTableShare.cpp 2008-08-16 22:04:29 +0000 @@ -108,6 +108,16 @@ void StorageTableShare::unlock(void) syncIndexes->unlock(); } +void StorageTableShare::lockIndexes(bool exclusiveLock) +{ + syncIndexes->lock(NULL, (exclusiveLock) ? Exclusive : Shared); +} + +void StorageTableShare::unlockIndexes(void) +{ + syncIndexes->unlock(); +} + int StorageTableShare::open(void) { if (!table) @@ -242,11 +252,11 @@ const char* StorageTableShare::cleanupTa return buffer; } -const char* StorageTableShare::createIndexName(const char *rawName, char *indexName) +char* StorageTableShare::createIndexName(const char *rawName, char *indexName) { - char nameBuffer[256]; + char nameBuffer[indexNameSize]; cleanupFieldName(rawName, nameBuffer, sizeof(nameBuffer)); - sprintf(indexName, "%s$%s", name, nameBuffer); + sprintf(indexName, "%s$%s", name.getString(), nameBuffer); return indexName; } @@ -255,6 +265,8 @@ int StorageTableShare::createIndex(Stora if (!table) open(); + // Always get syncIndexes before syncObject + Sync syncIndex(syncIndexes, "StorageTableShare::createIndex(1)"); syncIndex.lock(Exclusive); @@ -274,6 +286,8 @@ int StorageTableShare::dropIndex(Storage if (!table) open(); + // Always get syncIndexes before syncObject + Sync syncIndex(syncIndexes, "StorageTableShare::dropIndex(1)"); syncIndex.lock(Exclusive); @@ -342,8 +356,8 @@ int StorageTableShare::setIndex(int inde indexDesc->index = table->primaryKey; else { - char indexName[256]; - sprintf(indexName, "%s$%s", name, indexDesc->name); + char indexName[indexNameSize]; + sprintf(indexName, "%s$%s", name.getString(), indexDesc->name.getString()); indexDesc->index = table->findIndex(indexName); } @@ -366,7 +380,7 @@ void StorageTableShare::clearIndex(Stora for (int n = indexDesc->id; n < numberIndexes-1; n++) { indexes.vector[n] = indexes.vector[n+1]; - indexes.vector[n]->id = n; //cwp: assume that index id will match server + indexes.vector[n]->id = n; // assume that index id will match server } indexes.zap(numberIndexes-1); @@ -456,21 +470,6 @@ int StorageTableShare::setSequenceValue( return 0; } -// Get index id using the external (server) index name - -int StorageTableShare::getIndexId(const char* indexName) -{ - Sync sync(syncObject, "StorageTableShare::getIndexId(indexName)"); - sync.lock(Shared); - - if (indexes.length > 0) - for (int n = 0; n < numberIndexes; ++n) - if (strcmp(indexes.get(n)->name, indexName) == 0) - return n; - - return -1; -} - // Get index id using the internal (Falcon) index name int StorageTableShare::getIndexId(const char* schemaName, const char* indexName) === modified file 'storage/falcon/StorageTableShare.h' --- a/storage/falcon/StorageTableShare.h 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/StorageTableShare.h 2008-08-16 22:04:29 +0000 @@ -27,6 +27,7 @@ typedef __int64 INT64; static const int MaxIndexSegments = 16; +static const int indexNameSize = 257; class StorageDatabase; class StorageConnection; @@ -104,6 +105,8 @@ public: virtual void lock(bool exclusiveLock=false); virtual void unlock(void); + virtual void lockIndexes(bool exclusiveLock=false); + virtual void unlockIndexes(void); virtual int createIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, int indexCount, const char *sql); virtual int dropIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, const char *sql); virtual int renameTable(StorageConnection *storageConnection, const char* newName); @@ -122,7 +125,6 @@ public: StorageIndexDesc* getIndex(int indexId); StorageIndexDesc* getIndex(int indexId, StorageIndexDesc *indexDesc); StorageIndexDesc* getIndex(const char *name); - int getIndexId(const char* indexName); int getIndexId(const char* schemaName, const char* indexName); int create(StorageConnection *storageConnection, const char* sql, int64 autoIncrementValue); int upgrade(StorageConnection *storageConnection, const char* sql, int64 autoIncrementValue); @@ -141,7 +143,7 @@ public: static const char* getDefaultRoot(void); static const char* cleanupTableName(const char* name, char* buffer, int bufferLength, char *schema, int schemaLength); - const char* createIndexName(const char *rawName, char *indexName); + char* createIndexName(const char *rawName, char *indexName); JString name; JString schemaName; === modified file 'storage/falcon/Table.cpp' --- a/storage/falcon/Table.cpp 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/Table.cpp 2008-08-16 22:04:29 +0000 @@ -226,8 +226,11 @@ void Table::renameIndexes(const char *ne { if (index->type != PrimaryKey) { + + // Assume that index name is $ + char newIndexName[256]; - const char *p = strchr((const char*)index->name, '$'); //cwp check this naming assumption or better: use old table name + const char *p = strchr((const char*)index->name, '$'); sprintf(newIndexName, "%s%s", newTableName, (const char *)p); index->rename(newIndexName); } === modified file 'storage/falcon/ha_falcon.cpp' --- a/storage/falcon/ha_falcon.cpp 2008-08-16 18:06:36 +0000 +++ b/storage/falcon/ha_falcon.cpp 2008-08-16 22:04:29 +0000 @@ -398,6 +398,7 @@ StorageInterface::StorageInterface(handl ref_length = sizeof(lastRecord); stats.records = 1000; stats.data_file_length = 10000; + stats.index_file_length = 0; tableLocked = false; lockForUpdate = false; storageTable = NULL; @@ -485,6 +486,7 @@ int StorageInterface::open(const char *n if (!storageShare->initialized) { +// storageShare->lockIndexes(true); storageShare->lock(true); if (!storageShare->initialized) @@ -508,6 +510,7 @@ int StorageInterface::open(const char *n } storageShare->unlock(); +// storageShare->unlockIndexes(); } } @@ -679,7 +682,7 @@ void StorageInterface::getDemographics(v stats.block_size = 4096; - storageShare->lock(); + storageShare->lockIndexes(); for (uint n = 0; n < table->s->keys; ++n) { @@ -698,7 +701,7 @@ void StorageInterface::getDemographics(v } } - storageShare->unlock(); + storageShare->unlockIndexes(); DBUG_VOID_RETURN; } @@ -858,19 +861,19 @@ int StorageInterface::create(const char int StorageInterface::add_index(TABLE* table_arg, KEY* key_info, uint num_of_keys) { DBUG_ENTER("StorageInterface::add_index"); - int ret = createIndex(storageTable->getSchemaName(), storageTable->getName(), table_arg, table_arg->s->keys); // cwp bug + int ret = createIndex(storageTable->getSchemaName(), storageTable->getName(), table_arg, table_arg->s->keys); DBUG_RETURN(ret); } -int StorageInterface::createIndex(const char *schemaName, const char *tableName, TABLE *table, int indexId) //cwp why pass schema and tablename? +int StorageInterface::createIndex(const char *schemaName, const char *tableName, TABLE *table, int indexId) { KEY *key = table->key_info + indexId; StorageIndexDesc indexDesc(indexId); getKeyDesc(table, indexId, &indexDesc); - char indexName[256]; - storageShare->createIndexName(indexDesc.name, indexName); + char indexName[indexNameSize]; + storageShare->createIndexName(indexDesc.name.getString(), indexName); CmdGen gen; const char *unique = (key->flags & HA_NOSAME) ? "unique " : ""; @@ -883,11 +886,10 @@ int StorageInterface::createIndex(const int StorageInterface::dropIndex(const char *schemaName, const char *tableName, TABLE *table, int indexId) { - KEY *key = table->key_info + indexId; StorageIndexDesc indexDesc(indexId); getKeyDesc(table, indexId, &indexDesc); - char indexName[256]; + char indexName[indexNameSize]; storageShare->createIndexName(indexDesc.name, indexName); CmdGen gen; @@ -985,6 +987,7 @@ int StorageInterface::delete_table(const if (storageShare) { +// storageShare->lockIndexes(true); storageShare->lock(true); if (storageShare->initialized) @@ -995,6 +998,7 @@ int StorageInterface::delete_table(const } storageShare->unlock(); +// storageShare->unlockIndexes(); } int res = storageTable->deleteTable(); @@ -1380,7 +1384,6 @@ int StorageInterface::index_read(uchar * } } - int StorageInterface::index_init(uint idx, bool sorted) { DBUG_ENTER("StorageInterface::index_init"); @@ -1391,14 +1394,18 @@ int StorageInterface::index_init(uint id int ret = storageTable->setCurrentIndex(idx); + if (ret) DBUG_RETURN(error(ret)); -} + DBUG_RETURN(ret); +} int StorageInterface::index_end(void) { DBUG_ENTER("StorageInterface::index_end"); + storageTable->indexEnd(); + DBUG_RETURN(0); } @@ -1449,22 +1456,23 @@ void StorageInterface::getKeyDesc(TABLE { KEY *keyInfo = table->key_info + indexId; int numberKeys = keyInfo->key_parts; - char nameBuffer[256]; + char nameBuffer[indexNameSize]; indexDesc->rawName = keyInfo->name; - indexDesc->name = storageShare->cleanupFieldName(indexDesc->rawName, nameBuffer, sizeof(nameBuffer)); + storageShare->cleanupFieldName(indexDesc->rawName, nameBuffer, sizeof(nameBuffer)); + indexDesc->name = nameBuffer; indexDesc->numberSegments = numberKeys; indexDesc->unique = (keyInfo->flags & HA_NOSAME); - indexDesc->primaryKey = (table->s->primary_key == indexId); + indexDesc->primaryKey = (table->s->primary_key == (uint)indexId); for (int n = 0; n < numberKeys; ++n) { StorageSegment *segment = indexDesc->segments + n; KEY_PART_INFO *part = keyInfo->key_part + n; - segment->offset = part->offset; - segment->length = part->length; - segment->type = part->field->key_type(); + segment->offset = part->offset; + segment->length = part->length; + segment->type = part->field->key_type(); segment->nullBit = part->null_bit; segment->isUnsigned = (part->field->flags & ENUM_FLAG) ? true : ((Field_num*) part->field)->unsigned_flag; @@ -1485,7 +1493,6 @@ void StorageInterface::getKeyDesc(TABLE } } - int StorageInterface::rename_table(const char *from, const char *to) { DBUG_ENTER("StorageInterface::rename_table"); @@ -1502,17 +1509,14 @@ int StorageInterface::rename_table(const DBUG_RETURN(error(ret)); DBUG_RETURN(ret); - } - double StorageInterface::read_time(uint index, uint ranges, ha_rows rows) { DBUG_ENTER("StorageInterface::read_time"); DBUG_RETURN(rows2double(rows / 3)); } - int StorageInterface::read_range_first(const key_range *start_key, const key_range *end_key, bool eq_range_arg, bool sorted) @@ -1647,7 +1651,6 @@ int StorageInterface::index_next(uchar * } } - int StorageInterface::index_next_same(uchar *buf, const uchar *key, uint key_len) { DBUG_ENTER("StorageInterface::index_next_same"); @@ -1667,7 +1670,6 @@ int StorageInterface::index_next_same(uc } } - double StorageInterface::scan_time(void) { DBUG_ENTER("StorageInterface::scan_time"); @@ -1704,13 +1706,11 @@ bool StorageInterface::threadSwitch(THD* return true; } - int StorageInterface::threadSwitchError(void) { return 1; } - int StorageInterface::error(int storageError) { DBUG_ENTER("StorageInterface::error"); @@ -1750,7 +1750,6 @@ int StorageInterface::error(int storageE DBUG_RETURN(mySqlError); } - int StorageInterface::getMySqlError(int storageError) { switch (storageError) @@ -1938,7 +1937,7 @@ int StorageInterface::external_lock(THD if (storageConnection->startImplicitTransaction(isolation)) trans_register_ha(thd, false, falcon_hton); - } + } switch (thd_tx_isolation(mySqlThread)) { @@ -1950,13 +1949,11 @@ int StorageInterface::external_lock(THD error(StorageWarningSerializable); break; } - } DBUG_RETURN(0); } - void StorageInterface::get_auto_increment(ulonglong offset, ulonglong increment, ulonglong nb_desired_values, ulonglong *first_value, @@ -1969,7 +1966,6 @@ void StorageInterface::get_auto_incremen DBUG_VOID_RETURN; } - const char *StorageInterface::index_type(uint key_number) { DBUG_ENTER("StorageInterface::index_type"); @@ -1984,7 +1980,6 @@ void StorageInterface::dropDatabase(hand DBUG_VOID_RETURN; } - void StorageInterface::freeActiveBlobs(void) { for (StorageBlob *blob; (blob = activeBlobs); ) @@ -1996,14 +1991,12 @@ void StorageInterface::freeActiveBlobs(v } } - void StorageInterface::shutdown(handlerton *htons) { if(storageHandler) storageHandler->shutdownHandler(); } - int StorageInterface::panic(handlerton* hton, ha_panic_function flag) { if(storageHandler) @@ -2012,7 +2005,6 @@ int StorageInterface::panic(handlerton* return 0; } - int StorageInterface::closeConnection(handlerton *hton, THD *thd) { DBUG_ENTER("NfsStorageEngine::closeConnection"); @@ -2022,7 +2014,6 @@ int StorageInterface::closeConnection(ha DBUG_RETURN(0); } - int StorageInterface::alter_tablespace(handlerton* hton, THD* thd, st_alter_tablespace* ts_info) { DBUG_ENTER("NfsStorageEngine::alter_tablespace"); @@ -2302,7 +2293,6 @@ uint StorageInterface::max_supported_key return MAX_INDEX_KEY_LENGTH_4K; // Default value. } - uint StorageInterface::max_supported_key_part_length(void) const { // Assume 4K page unless proven otherwise. @@ -2312,7 +2302,6 @@ uint StorageInterface::max_supported_key return MAX_INDEX_KEY_LENGTH_4K; // Default for future sizes. } - void StorageInterface::logger(int mask, const char* text, void* arg) { if (mask & falcon_debug_mask) @@ -2343,17 +2332,17 @@ int StorageInterface::setIndexes(void) if (!table || storageShare->haveIndexes(table->s->keys)) return ret; - storageShare->lock(true); + storageShare->lockIndexes(true); if (!storageShare->haveIndexes(table->s->keys)) for (uint n = 0; n < table->s->keys; ++n) - if (ret = setIndex(table, n)) + if ((ret = setIndex(table, n))) break; - storageShare->unlock(); + storageShare->unlockIndexes(); - return ret; - } + return ret; +} int StorageInterface::genTable(TABLE* table, CmdGen* gen) { @@ -2508,7 +2497,6 @@ int StorageInterface::genType(Field* fie return 0; } - void StorageInterface::genKeyFields(KEY* key, CmdGen* gen) { const char *sep = "("; @@ -2518,8 +2506,7 @@ void StorageInterface::genKeyFields(KEY* { KEY_PART_INFO *part = key->key_part + n; Field *field = part->field; - storageShare->cleanupFieldName(field->field_name, nameBuffer, - sizeof(nameBuffer)); + storageShare->cleanupFieldName(field->field_name, nameBuffer, sizeof(nameBuffer)); if (part->key_part_flag & HA_PART_KEY_SEG) gen->gen("%s\"%s\"(%d)", sep, nameBuffer, part->length); @@ -2532,7 +2519,6 @@ void StorageInterface::genKeyFields(KEY* gen->gen(")"); } - void StorageInterface::encodeRecord(uchar *buf, bool updateFlag) { storageTable->preInsert();