Re: LRU - Mailing list pgsql-patches

On Mon, 2005-01-24 at 17:39 -0500, Tom Lane wrote:
> I'm more than slightly uncomfortable with the size of this patch.
[...]
> I think it would be better and safer to try to localize the changes
> into freelist.c.

IMHO that is basically what the patch does; the changes to other files
are either cosmetic or are required to change the replacement strategy.
More specifically, diffstat shows:

 doc/src/sgml/runtime.sgml                     |   48 -
 src/backend/catalog/index.c                   |    2
 src/backend/commands/dbcommands.c             |    4
 src/backend/commands/vacuum.c                 |   27
 src/backend/postmaster/bgwriter.c             |    3
 src/backend/storage/buffer/buf_init.c         |   55 !
 src/backend/storage/buffer/buf_table.c        |   67 !
 src/backend/storage/buffer/bufmgr.c           |  177 -!!
 src/backend/storage/buffer/freelist.c         |
1031 !!!!!!!!!!!!!!!!!!!!!!!!!
 src/backend/utils/misc/guc.c                  |   19
 src/backend/utils/misc/postgresql.conf.sample |    1
 src/include/postmaster/bgwriter.h             |    1
 src/include/storage/buf_internals.h           |   85 !!
 src/include/storage/bufmgr.h                  |    2
 14 files changed, 56 insertions(+), 139 deletions(-), 1327
modifications(!)

A summary of the changes made to each file:

runtime.sgml: doc updates
index.c: trivial API change (BufferSync)
dbcommands.c: trivial API change (BufferSync)
vacuum.c: trivial API change (StrategyVacuumHint)
bgwriter.c: trivial API change (BufferSync), remove bgwriter_percent
buf_init.c: changes required for LRU
buf_table.c: changes required for LRU (or more properly, reverting some
modifications to the buf_table stuff that was part of the ARC patch: the
patch should be very close to the buf_table in 7.4)
bufmgr.c: move PinBuffer() and UnpinBuffer() to freelist.c, replace
"StrategyXXX" calls with similar calls to BufTableXXX and so on. Some
BufferSync API changes. Most of these changes are pretty
straightforward.
freelist.c: obvious :)
guc.c: remove bgwriter_percent and debug_shared_buffers
postgresql.conf.sample: remove bgwriter_percent
bgwriter.h: remove bgwriter_percent
buf_internals.h: changes necessary for LRU
bufmgr.h: BufferSync API change

In other words, you might be able to somewhat reduce the size of the
patch by, say, not renaming some exported functions in freelist.c, but I
don't see that that will have a significant effect upon the complexity
of the patch or the risk that it might cause regressions. If you have
specific suggestions about how to reduce the scope of the patch I'm all
ears, but I don't think the size of the patch is necessarily the most
reliable metric for the probability it will result in a regression.

-Neil