Vlad,
Good work here. I have one question and a suggestion.
The suggestion is to use
else if (earlyWrite)
instead of
else if (!earlyWrite)
Switch them around so the code is testing the positive state instead of
the negative.
The question is about the (control->version < srlVersion20) condition in
SRLOverflowPages::pass2(void);
> + // We do not really know if this page is on disk
> + // or not, because early writeFlag is not available
> + // Don't delete it during recovery, otherwise data
> + // may be lost.
> + log->setOverflowPageInvalid(pageNumber, tableSpaceId);
Does making this overflow page invalid keep it from being used?
Vladislav Vaintroub wrote:
> #At file:///G:/bzr/mysql-6.0-falcon-team/ based on
> revid:vvaintroub@stripped
>
> 3131 Vladislav Vaintroub 2009-04-21
> Addendum to Bug #41840 Unbounded tablespace growth during recovery
>
> Problem: blob recovery might access invalid (free) page
> The reason is that page allocations are not recorded during recovery and
> recovery might assume that the last state (incarnation) of the page was "free", even if a
> piece of blob was written to it.
> Solution is to record blob pages (data or overflow pages or written with
> earlyWrite flag) but do nothing in recovery except incrementing the page incarnation. This
> way the page cannot not be accidentially freed.
>
> modified:
> storage/falcon/DataPage.cpp
> storage/falcon/Index.cpp
> storage/falcon/RecoveryObjects.h
> storage/falcon/SRLBlobUpdate.cpp
> storage/falcon/SRLOverflowPages.cpp
> storage/falcon/SRLOverflowPages.h
> storage/falcon/SRLVersion.h
> storage/falcon/Section.cpp
> === modified file 'storage/falcon/DataPage.cpp'
> --- a/storage/falcon/DataPage.cpp 2009-04-20 18:40:49 +0000
> +++ b/storage/falcon/DataPage.cpp 2009-04-21 10:43:08 +0000
> @@ -343,16 +343,14 @@ void DataPage::deleteOverflowPages(Dbb *
> {
> SerialLog *log = dbb->serialLog;
>
> - if (log->recovering)
> - {
> - if (!log->isOverflowPageValid(overflowPageNumber, dbb->tableSpaceId))
> - return;
> - log->setOverflowPageInvalid(overflowPageNumber, dbb->tableSpaceId);
> - }
> -
> -
> while (overflowPageNumber)
> {
> + if (log->recovering)
> + {
> + if (!log->isOverflowPageValid(overflowPageNumber, dbb->tableSpaceId))
> + return;
> + log->setOverflowPageInvalid(overflowPageNumber, dbb->tableSpaceId);
> + }
> Bdb *bdb = dbb->fetchPage (overflowPageNumber, PAGE_data_overflow, Exclusive);
> BDB_HISTORY(bdb);
> DataOverflowPage *page = (DataOverflowPage*) bdb->buffer;
>
> === modified file 'storage/falcon/Index.cpp'
> --- a/storage/falcon/Index.cpp 2009-04-08 15:36:49 +0000
> +++ b/storage/falcon/Index.cpp 2009-04-21 10:43:08 +0000
> @@ -375,7 +375,7 @@ void Index::makeKey(Field *field, Value
> size_t moveLen = MIN(indexKey->keyLength, MAX_PHYSICAL_KEY_LENGTH - 1);
> memmove (indexKey->key + 1, indexKey->key, moveLen);
> indexKey->key[0] = 0;
> - indexKey->keyLength = moveLen + 1;
> + indexKey->keyLength = (uint)moveLen + 1;
> }
> }
>
>
> === modified file 'storage/falcon/RecoveryObjects.h'
> --- a/storage/falcon/RecoveryObjects.h 2008-10-29 23:25:13 +0000
> +++ b/storage/falcon/RecoveryObjects.h 2009-04-21 10:43:08 +0000
> @@ -24,7 +24,7 @@
> #pragma once
> #endif // _MSC_VER > 1000
>
> -static const int RPG_HASH_SIZE = 101;
> +static const int RPG_HASH_SIZE = 1999;
>
> #include "SyncObject.h"
> class RecoveryPage;
>
> === modified file 'storage/falcon/SRLBlobUpdate.cpp'
> --- a/storage/falcon/SRLBlobUpdate.cpp 2007-11-01 20:28:51 +0000
> +++ b/storage/falcon/SRLBlobUpdate.cpp 2009-04-21 10:43:08 +0000
> @@ -76,14 +76,7 @@ void SRLBlobUpdate::pass1(void)
>
> void SRLBlobUpdate::pass2(void)
> {
> - if (transactionId == 11847)
> - print();
>
> - /***
> - if (log->tracePage == locatorPage || (dataPage && log->tracePage ==
> dataPage))
> - print();
> - ***/
> -
> bool ret1 = log->bumpPageIncarnation(locatorPage, tableSpaceId, objInUse);
> bool ret2 = log->bumpPageIncarnation(dataPage, tableSpaceId, objInUse);
>
>
> === modified file 'storage/falcon/SRLOverflowPages.cpp'
> --- a/storage/falcon/SRLOverflowPages.cpp 2009-04-20 18:40:49 +0000
> +++ b/storage/falcon/SRLOverflowPages.cpp 2009-04-21 10:43:08 +0000
> @@ -28,12 +28,13 @@ SRLOverflowPages::~SRLOverflowPages(void
> {
> }
>
> -void SRLOverflowPages::append(Dbb *dbb, Bitmap* pageNumbers)
> +void SRLOverflowPages::append(Dbb *dbb, Bitmap* pageNumbers, bool earlyWriteFlag)
> {
> for (int pageNumber = 0; pageNumber >= 0;)
> {
> START_RECORD(srlOverflowPages, "SRLOverflowPages::append");
> putInt(dbb->tableSpaceId);
> + putInt(earlyWriteFlag);
> UCHAR *lengthPtr = putFixedInt(0);
> UCHAR *start = log->writePtr;
> UCHAR *end = log->writeWarningTrack;
> @@ -63,6 +64,11 @@ void SRLOverflowPages::read(void)
> else
> tableSpaceId = 0;
>
> + if (control->version >=srlVersion20)
> + earlyWrite = getInt();
> + else
> + earlyWrite = 0;
> +
> dataLength = getInt();
> data = getData(dataLength);
> }
> @@ -91,8 +97,29 @@ void SRLOverflowPages::pass2(void)
>
> if (log->bumpPageIncarnation(pageNumber, tableSpaceId, objInUse))
> {
> - log->redoFreePage(pageNumber, tableSpaceId);
> - log->setOverflowPageInvalid(pageNumber, tableSpaceId);
> + if (control->version < srlVersion20)
> + {
> + // We do not really know if this page is on disk
> + // or not, because early writeFlag is not available
> + // Don't delete it during recovery, otherwise data
> + // may be lost.
> + log->setOverflowPageInvalid(pageNumber, tableSpaceId);
> + }
> + else
> + {
> + if (earlyWrite)
> + {
> + // It is a blob, and page is flushed to disk
> + log->setOverflowPageValid(pageNumber, tableSpaceId);
> + }
> + else
> + {
> + // Overflow page will be recreated, delete it to prevent
> + // lost pages.
> + log->redoFreePage(pageNumber, tableSpaceId);
> + log->setOverflowPageInvalid(pageNumber, tableSpaceId);
> + }
> + }
> }
> }
> }
>
> === modified file 'storage/falcon/SRLOverflowPages.h'
> --- a/storage/falcon/SRLOverflowPages.h 2008-11-14 02:30:11 +0000
> +++ b/storage/falcon/SRLOverflowPages.h 2009-04-21 10:43:08 +0000
> @@ -25,13 +25,13 @@ class SRLOverflowPages : public SerialLo
> public:
> SRLOverflowPages(void);
> ~SRLOverflowPages(void);
> - void append(Dbb *dbb, Bitmap* pageNumbers);
> + void append(Dbb *dbb, Bitmap* pageNumbers, bool earlyWriteFlag);
> virtual void read(void);
> virtual void pass1(void);
> virtual void pass2(void);
> virtual void redo(void);
> virtual void print(void);
> -
> + int earlyWrite;
> int dataLength;
> const UCHAR* data;
> };
>
> === modified file 'storage/falcon/SRLVersion.h'
> --- a/storage/falcon/SRLVersion.h 2009-03-02 18:36:32 +0000
> +++ b/storage/falcon/SRLVersion.h 2009-04-21 10:43:08 +0000
> @@ -46,7 +46,8 @@ static const int srlVersion16 = 16; //
> static const int srlVersion17 = 17; // Log root page number in SRLCreateIndex
> static const int srlVersion18 = 18; // Log tablespace list in SRLTableSpaces
> static const int srlVersion19 = 19; // Remove parent and prior pointers from
> SRLIndexPage
> -static const int srlCurrentVersion = srlVersion19;
> +static const int srlVersion20 = 20; // Add earlyWrite to SRLOverflowPages
> +static const int srlCurrentVersion = srlVersion20;
>
> class SRLVersion : public SerialLogRecord
> {
>
> === modified file 'storage/falcon/Section.cpp'
> --- a/storage/falcon/Section.cpp 2009-04-20 18:40:49 +0000
> +++ b/storage/falcon/Section.cpp 2009-04-21 10:43:08 +0000
> @@ -788,8 +788,8 @@ int Section::storeTail(Stream * stream,
> {
> if (log->recovering)
> log->setOverflowPageValid(overflowPageNumber, dbb->tableSpaceSectionId);
> - else if (!earlyWrite)
> - log->logControl->overflowPages.append(dbb, &pageNumbers);
> + else
> + log->logControl->overflowPages.append(dbb, &pageNumbers, earlyWrite);
> }
>
> return overflowPageNumber;
>
>
>
> ------------------------------------------------------------------------
>
>
