Re: patch for new feature: Buffer Cache Hibernation

Mitsuru IWASAKI wrote:
> the patch is available at:
> http://people.freebsd.org/~iwasaki/postgres/buffer-cache-hibernation-postgresql-20110508.patch 
>   

We can't accept patches just based on a pointer to a web site.  Please 
e-mail this to the mailing list so that it can be considered a 
submission under the project's licensing terms.

> I hope this would be committable and the final version.
>   

PostgreSQL has high standards for code submissions.  Extremely few 
submissions are committed without significant revisions to them based on 
code review.  So far you've gotten a first round of high-level design 
review, there's several additional steps before something is considered 
for a commit.  The whole process is outlined at 
http://wiki.postgresql.org/wiki/Submitting_a_Patch
From a couple of minutes of reading the patch, the first things that 
pop out as problems are:

-All of the ControlFile -> controlFile renaming has add a larger 
difference to ReadControlFile than I would consider ideal.
-Touching StrategyControl is not something this patch should be doing.
-I don't think your justification ("debugging or portability") for 
keeping around your original code in here is going to be sufficient to 
do so.
-This should not be named enable_buffer_cache_hibernation.  That very 
large diff you ended up with in the regression tests is because all of 
the settings named enable_* are optimizer control settings.  Using the 
name "buffer_cache_hibernation" instead would make a better starting point.
From a bigger picture perspective, this really hasn't addressed any of 
my comments about shared_buffers only being the beginning of the useful 
cache state to worry about here.  I'd at least like the solution to the 
buffer cache save/restore to have a plan for how it might address that 
too one day.  This project is also picky about only committing code that 
fits into the long-term picture for desired features.

Having a working example of a server-side feature doing cache storage 
and restoration is helpful though.  Don't think your work here is 
unappreciated--it is.  Getting this feature added is just a harder 
problem than what you've done so far.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us