From: | Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, pgsql-general(at)postgresql(dot)org, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: [PATCHES] A way to let Vacuum warn if FSM settings are low. |
Date: | 2005-02-24 21:52:59 |
Message-ID: | Pine.LNX.4.58.0502241337220.13185@greenie.cheapcomplexdevices.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-patches |
Thanks everyone for the feedback on my patch.
Objections I've heard (both online and in email) included:
* WARNING is too strong for possibly OK behavior
* It's similar to "checkpoints occuring too frequently...
consider increasing...checkpoint_segments" which
is a LOG not a WARNING.
* The end user can't do anything anyway; so the LOG
file would be a better place.
* My comparison for numRels was broken.
* If we're hiting the user to do something (change
settings) I should make it put a HINT in the log file.
Praise I've heard included:
* Even if it's too conservative, some admins want to know.
* Unlike the current VACUUM VERBOSE info, all info is on
one line, so automated log monitoring software can more
easily catch it.
* Unlike the current VACUUM VERBOSE info, this one points
the user in the right direction.
Would the updated patch below address most of the concerns?
The output now looks like:
LOG: max_fsm_pages(1601) is smaller than the actual number of page slots needed(2832)
HINT: You may want to increase max_fsm_pages to be larger than 2832
and only goes in the log file (like the "checkpoints" hint).
I think Tom's outstanding comment that "Depending on the installation,
this might be a perfectly acceptable situation ... I don't know how
toproduce a warning about MaxFSMPages that's worth anything" is the
only objection left unaddressed. I guess my defense to that statement
would be that I think for some installations this does provide value,
so by making it a LOG instead of a WARNING are both needs met?
Thanks,
Ron
============================================================
% diff -U 10 postgresql-8.0.1/src/backend/storage/freespace/freespace.c postgresql-patched/src/backend/storage/freespace/freespace.c
--- postgresql-8.0.1/src/backend/storage/freespace/freespace.c 2004-12-31 14:00:54.000000000 -0800
+++ postgresql-patched/src/backend/storage/freespace/freespace.c 2005-02-24 13:44:52.361669928 -0800
@@ -704,20 +704,32 @@
/* Convert stats to actual number of page slots needed */
needed = (sumRequests + numRels) * CHUNKPAGES;
ereport(elevel,
(errmsg("free space map: %d relations, %d pages stored; %.0f total pages needed",
numRels, storedPages, needed),
errdetail("Allocated FSM size: %d relations + %d pages = %.0f kB shared memory.",
MaxFSMRelations, MaxFSMPages,
(double) FreeSpaceShmemSize() / 1024.0)));
+
+ if (needed > MaxFSMPages)
+ ereport(LOG,
+ (errmsg("max_fsm_pages(%d) is smaller than the actual number of page slots needed(%.0f)",
+ MaxFSMPages, needed),
+ errhint("You may want to increase max_fsm_pages to be larger than %.0f",needed)));
+ if (numRels == MaxFSMRelations)
+ ereport(LOG,
+ (errmsg("max_fsm_relations(%d) is equal than the number of relations vacuum checked (%d)",
+ MaxFSMRelations, numRels),
+ errhint("You probably have more than %d relations. You should increase max_fsm_relations. Pages needed for max_fsm_pages may have been underestimated as well. ",numRels)));
+
}
/*
* DumpFreeSpaceMap - dump contents of FSM into a disk file for later reload
*
* This is expected to be called during database shutdown, after updates to
* the FSM have stopped. We lock the FreeSpaceLock but that's purely pro
* forma --- if anyone else is still accessing FSM, there's a problem.
*/
void
============================================================
From | Date | Subject | |
---|---|---|---|
Next Message | Rick Casey | 2005-02-24 22:13:52 | Re: basic trigger using OLD not working? |
Previous Message | Rick Casey | 2005-02-24 21:22:20 | basic trigger using OLD not working? |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2005-02-24 22:27:25 | Re: [PATCHES] [pgsql-hackers-win32] Repleacement for src/port/snprintf.c |
Previous Message | Tom Lane | 2005-02-24 17:44:00 | Re: [ADMIN] invalid multibyte character for locale |