List:Falcon Storage Engine« Previous MessageNext Message »
From:Christopher Powers Date:October 17 2008 4:55pm
Subject:Re: Review patch for WL4573?
View as plain text  
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
> 
> 
> ------------------------------------------------------------------------
> 
> 
Thread
Review patch for WL4573?Ann W. Harrison17 Oct
  • Re: Review patch for WL4573?Christopher Powers17 Oct
    • Re: Review patch for WL4573?Ann W. Harrison17 Oct
Re: Review patch for WL4573?Christopher Powers17 Oct