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
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 |