From: Christopher Powers
Date: October 17 2008 5:05pm
Subject: Re: Review  patch for WL4573?
List-Archive: http://lists.mysql.com/falcon/46
Message-Id: <48F8C5F3.8010808@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 7BIT

Ann W. Harrison wrote:
> This was originally sent to Chris, but I realize that changes should
> have more than one reviewer ...
> 
> 
> I've coded and tested the changes described in WL4573.  The intention
> of the patch is to recognize and fix any pages that are fetched during
> recovery but aren't marked as in use on the PIP.  If Falcon fetches a
> page, it should be in use.  Although the patch is in a critical area,
> it's very simple, and will help us diagnose databases that were accessed
> by a version of Falcon that had the incorrect cache flush code.
> 
> The check should be done only during recovery because it will affect
> performance.  Until now, Cache didn't know that recovery was going on,
> so some code had to be added to Dbb to set a new boolean value in
> Cache indicating that recovery is in process.

Ann,

Just an observation: Calling PageInventoryPage::isPageInUse() from
Cache::fetchPage() results in a single recursive call to
Cache::fetchPage(). We are protected from infinite recursion (e.g. if
dbb->pagesPerPip == 1) and a blown stack because of the filter on
pageType != PAGE_Inventory.

Would it be worthwhile to combine PageInventoryPage::isPageInUse() and
::markPageInUse()?

May want to add a new line at the end of Dbb.cpp

Otherwise, the patch looks good.



> Best regards
> 
> Ann
> 
> 
> ------------------------------------------------------------------------
> 
>