Re: patch for new feature: Buffer Cache Hibernation

From: Greg Smith <greg(at)2ndquadrant(dot)com>
To: Mitsuru IWASAKI <iwasaki(at)jp(dot)FreeBSD(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org, robertmhaas(at)gmail(dot)com
Subject: Re: patch for new feature: Buffer Cache Hibernation
Date: 2011-05-08 06:41:38
Message-ID: 4DC63B22.1060009@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jean-Paul Argudo 2011-05-08 09:07:30 Re: New Canadian nonprofit for trademark, postgresql.org domain, etc.
Previous Message Leonardo Francalanci 2011-05-08 06:31:38 Re: switch UNLOGGED to LOGGED