2705 Kevin Lewis 2009-05-14
Bug#44850 - Delete the member variable StorageTable::dataStream since it can point into Storagetable::record.data.record, which can be chilled while it exists.
Instead, use local pointers that are protected by a CycleLock so that even if the data.record is chilled, that buffer will be accessable until the next cycle.
Fix Record::deleteData so that it works for chilled records.
Also, fix Record::getDataMemUsage() so that it does not thaw. That was an inadvertent change from a recent patch.
modified:
storage/falcon/Record.cpp
storage/falcon/StorageTable.cpp
storage/falcon/StorageTable.h
storage/falcon/ha_falcon.cpp
2704 Kevin Lewis 2009-05-12
Code Cleanup and compile warnings
Fix the previous patch for Bug#43344 & Bug#44721 that made a call to getAllocatedSize()
from inside a thaw. This caused a recursive loop and a stack overflow.
modified:
storage/falcon/Dbb.cpp
storage/falcon/Record.cpp
storage/falcon/Record.h
storage/falcon/RecordVersion.cpp
storage/falcon/SRLUpdateRecords.cpp
2703 Kevin Lewis 2009-05-12 [merge]
merge
modified:
storage/falcon/WalkIndex.cpp
2700.1.1 lars-erik.bjork@stripped 2009-05-09
This is one of at least two patches for bug#43630
'A SELECT using ORDER BY and LIMIT sometimes returns no rows'
The index walker logic does not handle if an index page is empty. In
loads with lots of updates, we may get empty pages. When walking the
index we will stop at the first empty index page, even though this is
not the end of the level. In this test, this has often been the first
page, resulting in no rows returned.
This patch makes the search proceed to the next page when an empty
index page that is not the end of the level is encountered.
=== modified file 'storage/falcon/WalkIndex.cpp'
WalkIndex::getNextNode has been changed to reposition the index to the
next page when an index page with BUCKET_END as it first node is
encountered.
modified:
storage/falcon/WalkIndex.cpp
=== modified file 'storage/falcon/Dbb.cpp'
--- a/storage/falcon/Dbb.cpp 2009-04-27 16:43:53 +0000
+++ b/storage/falcon/Dbb.cpp 2009-05-12 22:58:23 +0000
@@ -793,7 +793,7 @@ int64 Dbb::updateSequence(int sequenceId
int slot = sequenceId % sequencesPerPage;
int64 value;
- if (delta && page->sequences [slot] != 0xffffffffffffffffLL)
+ if (delta && page->sequences [slot] != (int64) 0xffffffffffffffffLL)
{
bdb->mark(transId);
=== modified file 'storage/falcon/Record.cpp'
--- a/storage/falcon/Record.cpp 2009-05-12 14:35:40 +0000
+++ b/storage/falcon/Record.cpp 2009-05-14 05:16:08 +0000
@@ -922,15 +922,11 @@ void Record::deleteData(bool now)
{
// Detach the pointer from data.record
- char* recordData = data.record;
- while (isAllocated(recordData))
- {
+ char* recordData;
+ while ((recordData = data.record))
if (COMPARE_EXCHANGE_POINTER(&data.record, recordData, NULL))
break;
- recordData = data.record;
- }
-
// Now free the detached buffer
if (isAllocated(recordData))
@@ -962,7 +958,6 @@ void Record::deleteData(bool now)
// Only one thread can chill a record at a time because that
// thread must 'own' the transaction. So this CAS may not be needed.
-
void Record::chillData(void)
{
// Detach the pointer from data.record
@@ -1097,9 +1092,9 @@ int Record::getSize(void)
int Record::getDataMemUsage(void)
{
- char* recordData = getRecordData();
+ char* recordData = findRecordData();
- return (recordData == NULL ? 0 : MemMgr::blockSize(recordData));
+ return (isAllocated(recordData) ? MemMgr::blockSize(recordData) : 0);
}
int Record::getMemUsage(void)
=== modified file 'storage/falcon/Record.h'
--- a/storage/falcon/Record.h 2009-05-12 14:35:40 +0000
+++ b/storage/falcon/Record.h 2009-05-12 22:58:23 +0000
@@ -180,7 +180,6 @@ protected:
return ((recordData != NULL) && (recordData != recordIsChilled));
}
-
public:
uint64 generation;
Format *format;
=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp 2009-05-12 14:35:40 +0000
+++ b/storage/falcon/RecordVersion.cpp 2009-05-12 22:58:23 +0000
@@ -434,7 +434,7 @@ char *RecordVersion::thaw()
if (table->dbb->fetchRecord(table->dataSection, recordNumber, &stream))
{
recordData = setEncodedRecord(&stream, true);
- bytesReallocated = getAllocatedSize();
+ bytesReallocated = size - getSize();
}
if (bytesReallocated > 0)
=== modified file 'storage/falcon/SRLUpdateRecords.cpp'
--- a/storage/falcon/SRLUpdateRecords.cpp 2009-05-12 14:35:40 +0000
+++ b/storage/falcon/SRLUpdateRecords.cpp 2009-05-12 22:58:23 +0000
@@ -43,7 +43,7 @@ bool SRLUpdateRecords::chill(Transaction
// Record data has been written to the serial log.
// Before releasing the data, find the allocated size.
- int chillBytes = record->getAllocatedSize();
+ uint64 chillBytes = record->getAllocatedSize();
// Release the data buffer and set the state accordingly
=== modified file 'storage/falcon/StorageTable.cpp'
--- a/storage/falcon/StorageTable.cpp 2009-04-16 11:25:48 +0000
+++ b/storage/falcon/StorageTable.cpp 2009-05-14 05:16:08 +0000
@@ -382,12 +382,21 @@ void StorageTable::setRecord(Record* new
record = newRecord;
recordLocked = locked;
format = record->format;
-
+}
+
+// Use the curent StorageTable::record to set up an EncodedDataStream
+// Treat this like a temporary pointer kept alive by the CycleManager
+// since this data.record could be chilled.
+
+void StorageTable::setData(EncodedDataStream* dataStream)
+{
+ ASSERT(record);
+
// The following is confusing because Record::getEncodeRecord returns pointer to
// the actual data fields while Record::getEncodedSize return length of the data fields plus
// a two byte format number.
- dataStream.setData((const UCHAR*) record->getEncodedRecord(), record->getEncodedSize() - sizeof(USHORT));
+ dataStream->setData((const UCHAR*) record->getEncodedRecord(), record->getEncodedSize() - sizeof(USHORT));
}
void StorageTable::clearRecord(void)
@@ -399,12 +408,12 @@ void StorageTable::clearRecord(void)
}
}
-void StorageTable::preInsert(void)
+void StorageTable::preInsert(EncodedDataStream* dataStream)
{
insertStream.clear();
short version = -share->table->getFormatVersion();
insertStream.putSegment(sizeof(version), (char*) &version, true);
- dataStream.setStream(&insertStream);
+ dataStream->setStream(&insertStream);
}
const UCHAR* StorageTable::getEncoding(int fieldIndex)
=== modified file 'storage/falcon/StorageTable.h'
--- a/storage/falcon/StorageTable.h 2009-03-20 19:28:10 +0000
+++ b/storage/falcon/StorageTable.h 2009-05-14 05:16:08 +0000
@@ -72,6 +72,7 @@ public:
void transactionEnded(void);
void setRecord(Record* record, bool locked);
+ void setData(EncodedDataStream* dataStream);
int alterCheck(void);
void waitForWriteComplete();
void clearAlter(void);
@@ -100,7 +101,7 @@ public:
virtual void release(StorageBlob* blob);
virtual void deleteStorageTable(void);
virtual void freeBlob(StorageBlob* blob);
- virtual void preInsert(void);
+ virtual void preInsert(EncodedDataStream* dataStream);
virtual int insert(void);
virtual int next(int recordNumber, bool lockForUpdate);
@@ -145,7 +146,6 @@ public:
StorageKey *upperBound;
Record *record;
Format *format;
- EncodedDataStream dataStream;
Stream insertStream;
int searchFlags;
bool recordLocked;
=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp 2009-05-06 19:40:24 +0000
+++ b/storage/falcon/ha_falcon.cpp 2009-05-14 05:16:08 +0000
@@ -37,6 +37,7 @@
#include "Error.h"
#include "Log.h"
#include "ErrorInjector.h"
+#include "CycleLock.h"
#ifdef _WIN32
#define I64FORMAT "%I64d"
@@ -2971,10 +2972,10 @@ void StorageInterface::genKeyFields(KEY*
void StorageInterface::encodeRecord(uchar *buf, bool updateFlag)
{
- storageTable->preInsert();
+ EncodedDataStream dataStream;
+ storageTable->preInsert(&dataStream);
my_ptrdiff_t ptrDiff = buf - table->record[0];
my_bitmap_map *old_map = dbug_tmp_use_all_columns(table, table->read_set);
- EncodedDataStream *dataStream = &storageTable->dataStream;
FieldFormat *fieldFormat = storageShare->format->format;
int maxId = storageShare->format->maxId;
@@ -2992,10 +2993,10 @@ void StorageInterface::encodeRecord(ucha
if (updateFlag && !bitmap_is_set(table->write_set, field->field_index))
{
const unsigned char *p = storageTable->getEncoding(n);
- dataStream->encodeEncoding(p);
+ dataStream.encodeEncoding(p);
}
else if (field->is_null())
- dataStream->encodeNull();
+ dataStream.encodeNull();
else
switch (field->real_type())
{
@@ -3007,7 +3008,7 @@ void StorageInterface::encodeRecord(ucha
case MYSQL_TYPE_ENUM:
case MYSQL_TYPE_SET:
case MYSQL_TYPE_BIT:
- dataStream->encodeInt64(field->val_int());
+ dataStream.encodeInt64(field->val_int());
break;
case MYSQL_TYPE_LONGLONG:
@@ -3022,11 +3023,11 @@ void StorageInterface::encodeRecord(ucha
{
BigInt bigInt;
bigInt.set((uint64)temp);
- dataStream->encodeBigInt(&bigInt);
+ dataStream.encodeBigInt(&bigInt);
}
else
{
- dataStream->encodeInt64(temp);
+ dataStream.encodeInt64(temp);
}
}
@@ -3035,7 +3036,7 @@ void StorageInterface::encodeRecord(ucha
case MYSQL_TYPE_YEAR:
// Have to use the ptr directly to get the same number for
// both two and four digit YEAR
- dataStream->encodeInt64((int) field->ptr[0]);
+ dataStream.encodeInt64((int) field->ptr[0]);
break;
case MYSQL_TYPE_NEWDECIMAL:
@@ -3048,7 +3049,7 @@ void StorageInterface::encodeRecord(ucha
int64 value = ScaledBinary::getInt64FromBinaryDecimal((const char *) field->ptr,
precision,
scale);
- dataStream->encodeInt64(value, scale);
+ dataStream.encodeInt64(value, scale);
}
else
{
@@ -3061,42 +3062,42 @@ void StorageInterface::encodeRecord(ucha
if (bigInt.fitsInInt64() && scale < 19)
{
int64 value = bigInt.getInt();
- dataStream->encodeInt64(value, scale);
+ dataStream.encodeInt64(value, scale);
}
else
- dataStream->encodeBigInt(&bigInt);
+ dataStream.encodeBigInt(&bigInt);
}
}
break;
case MYSQL_TYPE_DOUBLE:
case MYSQL_TYPE_FLOAT:
- dataStream->encodeDouble(field->val_real());
+ dataStream.encodeDouble(field->val_real());
break;
case MYSQL_TYPE_TIMESTAMP:
{
my_bool nullValue;
int64 value = ((Field_timestamp*) field)->get_timestamp(&nullValue);
- dataStream->encodeDate(value * 1000);
+ dataStream.encodeDate(value * 1000);
}
break;
case MYSQL_TYPE_DATE:
- dataStream->encodeInt64(field->val_int());
+ dataStream.encodeInt64(field->val_int());
break;
case MYSQL_TYPE_NEWDATE:
- //dataStream->encodeInt64(field->val_int());
- dataStream->encodeInt64(uint3korr(field->ptr));
+ //dataStream.encodeInt64(field->val_int());
+ dataStream.encodeInt64(uint3korr(field->ptr));
break;
case MYSQL_TYPE_TIME:
- dataStream->encodeInt64(field->val_int());
+ dataStream.encodeInt64(field->val_int());
break;
case MYSQL_TYPE_DATETIME:
- dataStream->encodeInt64(field->val_int());
+ dataStream.encodeInt64(field->val_int());
break;
case MYSQL_TYPE_VARCHAR:
@@ -3106,7 +3107,7 @@ void StorageInterface::encodeRecord(ucha
String string;
String buffer;
field->val_str(&buffer, &string);
- dataStream->encodeOpaque(string.length(), string.ptr());
+ dataStream.encodeOpaque(string.length(), string.ptr());
}
break;
@@ -3116,7 +3117,7 @@ void StorageInterface::encodeRecord(ucha
uint length = blob->get_length();
uchar *ptr;
blob->get_ptr(&ptr);
- dataStream->encodeOpaque(length, (const char*) ptr);
+ dataStream.encodeOpaque(length, (const char*) ptr);
}
break;
@@ -3148,12 +3149,12 @@ void StorageInterface::encodeRecord(ucha
blob->set_ptr(storageBlob.length, storageBlob.data);
}
- dataStream->encodeBinaryBlob(blobId);
+ dataStream.encodeBinaryBlob(blobId);
}
break;
default:
- dataStream->encodeOpaque(field->field_length, (const char*) field->ptr);
+ dataStream.encodeOpaque(field->field_length, (const char*) field->ptr);
}
if (ptrDiff)
@@ -3165,7 +3166,13 @@ void StorageInterface::encodeRecord(ucha
void StorageInterface::decodeRecord(uchar *buf)
{
- EncodedDataStream *dataStream = &storageTable->dataStream;
+ // dataStream will point into the data buffer of StorageTable::record
+ // Use a cycleLock to hold that buffer in case it is chilled.
+
+ CycleLock cycleLock(storageConnection->database);
+ EncodedDataStream dataStream;
+ storageTable->setData(&dataStream);
+
my_ptrdiff_t ptrDiff = buf - table->record[0];
my_bitmap_map *old_map = dbug_tmp_use_all_columns(table, table->write_set);
DBUG_ENTER("StorageInterface::decodeRecord");
@@ -3204,7 +3211,7 @@ void StorageInterface::decodeRecord(ucha
if (fieldFormat->fieldId < 0 || fieldFormat->offset == 0)
continue;
- dataStream->decode();
+ dataStream.decode();
Field *field = fieldMap[fieldFormat->fieldId];
// If we don't have a field for the physical field, just skip over it and don't worry
@@ -3215,7 +3222,7 @@ void StorageInterface::decodeRecord(ucha
if (ptrDiff)
field->move_field_offset(ptrDiff);
- if (dataStream->type == edsTypeNull || !bitmap_is_set(table->read_set, field->field_index))
+ if (dataStream.type == edsTypeNull || !bitmap_is_set(table->read_set, field->field_index))
{
field->set_null();
field->reset();
@@ -3234,7 +3241,7 @@ void StorageInterface::decodeRecord(ucha
case MYSQL_TYPE_ENUM:
case MYSQL_TYPE_SET:
case MYSQL_TYPE_BIT:
- field->store(dataStream->getInt64(),
+ field->store(dataStream.getInt64(),
((Field_num*)field)->unsigned_flag);
break;
@@ -3246,14 +3253,14 @@ void StorageInterface::decodeRecord(ucha
// support unsigned values with the MSB set in
// the index
- if (dataStream->type == edsTypeBigInt)
+ if (dataStream.type == edsTypeBigInt)
{
- int64 value = dataStream->bigInt.getInt();
+ int64 value = dataStream.bigInt.getInt();
field->store(value, ((Field_num*)field)->unsigned_flag);
}
else
{
- field->store(dataStream->getInt64(),
+ field->store(dataStream.getInt64(),
((Field_num*)field)->unsigned_flag);
}
}
@@ -3262,7 +3269,7 @@ void StorageInterface::decodeRecord(ucha
case MYSQL_TYPE_YEAR:
// Must add 1900 to give Field_year::store the value it
// expects. See also case 'MYSQL_TYPE_YEAR' in encodeRecord()
- field->store(dataStream->getInt64() + 1900, ((Field_num*)field)->unsigned_flag);
+ field->store(dataStream.getInt64() + 1900, ((Field_num*)field)->unsigned_flag);
break;
case MYSQL_TYPE_NEWDECIMAL:
@@ -3270,11 +3277,11 @@ void StorageInterface::decodeRecord(ucha
int precision = ((Field_new_decimal*) field)->precision;
int scale = ((Field_new_decimal*) field)->dec;
- if (dataStream->type == edsTypeBigInt)
- ScaledBinary::putBigInt(&dataStream->bigInt, (char*) field->ptr, precision, scale);
+ if (dataStream.type == edsTypeBigInt)
+ ScaledBinary::putBigInt(&dataStream.bigInt, (char*) field->ptr, precision, scale);
else
{
- int64 value = dataStream->getInt64(scale);
+ int64 value = dataStream.getInt64(scale);
ScaledBinary::putBinaryDecimal(value, (char*) field->ptr, precision, scale);
}
}
@@ -3282,44 +3289,44 @@ void StorageInterface::decodeRecord(ucha
case MYSQL_TYPE_DOUBLE:
case MYSQL_TYPE_FLOAT:
- field->store(dataStream->value.dbl);
+ field->store(dataStream.value.dbl);
break;
case MYSQL_TYPE_TIMESTAMP:
{
- int value = (int) (dataStream->value.integer64 / 1000);
+ int value = (int) (dataStream.value.integer64 / 1000);
((Field_timestamp*)field)->store_timestamp(value);
}
break;
case MYSQL_TYPE_DATE:
- field->store(dataStream->getInt64(), false);
+ field->store(dataStream.getInt64(), false);
break;
case MYSQL_TYPE_NEWDATE:
- int3store(field->ptr, dataStream->getInt32());
+ int3store(field->ptr, dataStream.getInt32());
break;
case MYSQL_TYPE_TIME:
- field->store(dataStream->getInt64(), false);
+ field->store(dataStream.getInt64(), false);
break;
case MYSQL_TYPE_DATETIME:
- int8store(field->ptr, dataStream->getInt64());
+ int8store(field->ptr, dataStream.getInt64());
break;
case MYSQL_TYPE_VARCHAR:
case MYSQL_TYPE_VAR_STRING:
case MYSQL_TYPE_STRING:
- field->store((const char*) dataStream->value.string.data,
- dataStream->value.string.length, field->charset());
+ field->store((const char*) dataStream.value.string.data,
+ dataStream.value.string.length, field->charset());
break;
case MYSQL_TYPE_TINY_BLOB:
{
Field_blob *blob = (Field_blob*) field;
- blob->set_ptr(dataStream->value.string.length,
- (uchar*) dataStream->value.string.data);
+ blob->set_ptr(dataStream.value.string.length,
+ (uchar*) dataStream.value.string.data);
}
break;
@@ -3338,7 +3345,7 @@ void StorageInterface::decodeRecord(ucha
storageBlob->next = activeBlobs;
activeBlobs = storageBlob;
- storageBlob->blobId = dataStream->value.blobId;
+ storageBlob->blobId = dataStream.value.blobId;
storageTable->getBlob(storageBlob->blobId, storageBlob);
blob->set_ptr(storageBlob->length, (uchar*) storageBlob->data);
}
@@ -3346,12 +3353,12 @@ void StorageInterface::decodeRecord(ucha
default:
{
- uint l = dataStream->value.string.length;
+ uint l = dataStream.value.string.length;
if (field->field_length < l)
l = field->field_length;
- memcpy(field->ptr, dataStream->value.string.data, l);
+ memcpy(field->ptr, dataStream.value.string.data, l);
}
}
}
Attachment: [text/bzr-bundle] bzr/kevin.lewis@sun.com-20090514051608-ivmrbyaf7akqaeqf.bundle
