Bug #47519 Falcon; Assert in StorageTable::updateRow
Submitted: 22 Sep 2009 13:01 Modified: 26 May 2010 17:48
Reporter: Kevin Lewis Email Updates:
Status: Unsupported Impact on me:
None 
Category:MySQL Server: Falcon storage engine Severity:S3 (Non-critical)
Version: OS:Any
Assigned to: Kevin Lewis CPU Architecture:Any
Tags: F_RECORD TREE

[22 Sep 2009 13:01] Kevin Lewis
Description:
With thisrecent introduction of intermediate record scavenging, an new assert has occurred once on my development machine;
 	
 	Error::debugBreak() Line 92	C++
 	Error::error(const char * string=0x00cabc3c, ...) Line 73	C++
 	Error::assertionFailed(const char * text=0x00cac7cc, const char * fileName=0x00cac7b8, int line=134) Line 78 + 0x16 bytes	C++
>	StorageTable::updateRow(int recordNumber=27705) Line 134 + 0x23 bytes	C++
 	StorageInterface::update_row(const unsigned char * oldData=0x0f24e6d0, unsigned char * newData=0x0f24bec0) Line 1241 + 0x25 bytes	C++
 	handler::ha_update_row(const unsigned char * old_data=0x0f24e6d0, unsigned char * new_data=0x0f24bec0) Line 5538 + 0x1a bytes	C++
 	mysql_update(THD * thd=0x0f29f4d0, TABLE_LIST * table_list=0x0f2a7a60, List<Item> & fields={...}, List<Item> & values={...}, Item * conds=0x0f2a8298, unsigned int order_num=0, st_order * order=0x00000000, unsigned __int64 limit=18446744073709551615, enum_duplicates handle_duplicates=DUP_ERROR, bool ignore=false) Line 651 + 0x19 bytes	C++
 	mysql_execute_command(THD * thd=0x0f29f4d0) Line 3142 + 0x61 bytes	C++
 	mysql_parse(THD * thd=0x0f29f4d0, const char * inBuf=0x0f2a75a8, unsigned int length=192, const char * * found_semicolon=0x11b1fcf8) Line 5979 + 0x9 bytes	C++
 	dispatch_command(enum_server_command command=COM_QUERY, THD * thd=0x0f29f4d0, char * packet=0x0f2a2531, unsigned int packet_length=192) Line 1064 + 0x1e bytes	C++
 	do_command(THD * thd=0x0f29f4d0) Line 746 + 0x1b bytes	C++
 	handle_one_connection(void * arg=0x0f29f4d0) Line 1146 + 0x9 bytes	C++
 	pthread_start(void * p=0x0f2c9fa8) Line 61 + 0x9 bytes	C

The assert that fails is this;

int StorageTable::updateRow(int recordNumber)
{
	try
		{
		ASSERT(record->useCount >= 2);
		storageDatabase->updateRow(storageConnection, share->table, record, &insertStream);
		}
	catch (SQLException& exception)

How to repeat:
It only occurred once after dozens of RQG runs of falcon_backlog.

Suggested fix:
The record->state == recQueuedForDelete.  So this record was snipped off the record chain and deleted while it still had a pointer to it from StorageTable::record.  The useCount of 1 is for that pointer.  This means that the algorithm for snipping is incorrect since it is supposed to snip only invisible records, and this one is obviously visible.
[22 Sep 2009 13:02] Kevin Lewis
Verified by developer.
[22 Sep 2009 13:21] Kevin Lewis
The algorithm for choosing a record to snip is wrong because it uses the transaction ID to determine if an old record is visible to an active transaction. It aught to use the commit ID of the old record.

Example Record chain:  (transaction ID - commit ID)
(5-9)<-(20-25)<-(30-45)<-(50-55)
Active Transactions = 10, 40 & 60

In this example, transaction 10 sees the record for transaction 5 since it committed before 10 started.  But transaction 40 does not see the record for transaction 30 since it was not committed when 40 started. Instead, it sees the record for transaction 20.  The current algorithm only looks at the transaction ID and so chooses (20-25) to snip.  Instead, it should snip (30-45) since that record is actually the only one invisible to any active transaction.

There is a function is Falcon called Transaction::visible() which does this correctly, but it was not used in RecordScavenge::canBeSnipped() because the pointer to the active transaction is not readily available.  It uses a bitmap of active transaction IDs.  The the rules implemented correctly in Transaction::visible() should be done again in RecordScavenge::canBeSnipped().
[28 Sep 2009 16:58] Kevin Lewis
Fixed with the following patch;

------------------------------------------------------------
revno: 2792
revision-id: kevin.lewis@sun.com-20090922145242-iqzj5jgj2fje7zzw
parent: kevin.lewis@sun.com-20090922134839-qfh7660uls7l982j
committer: Kevin Lewis <kevin.lewis@sun.com>
branch nick: mysql-6.0-falcon-team
timestamp: Tue 2009-09-22 09:52:42 -0500
message:
  Bug#47519 - Implemented an improved version of
  RecordScavenge::canBeSnipped() which uses the commitId
  to test visibility instead of the transactionId.
  This makes it more like Transaction::visible.
------------------------------------------------------------