Thread: A way to let Vacuum warn if FSM settings are low.
Short summary:
I find this tiny (9-line) patch useful to help my clients know
when FSM settings may need updating.
Some of the more frequently asked questions here are in regards to FSM
settings. One hint I've seen is to run "vacuum verbose;". At the end
of thousands of lines of INFO and DETAIL messages vacuum verbose has 2
separate lines with some numbers to compare ("total pages needed" and
"FSM size...pages") that help indicate too low fsm settings.
I've gotten into the habit of always installing the following patch
(below) that automatically does this comparison for me, and if
max_fsm_pages is too small, it logs a warning as shown here:
patched=# vacuum;
WARNING: max_fsm_pages(1601) is smaller than total pages needed(2832)
VACUUM
I find this much nicer than the existing output (
clean=# vacuum verbose;
[......... thousands of lines of INFO and DETAIL messages ........]
INFO: free space map: 77 relations, 470 pages stored; 2832 total pages needed
DETAIL: Allocated FSM size: 100 relations + 1601 pages = 19 kB shared memory.
) for many reasons:
* First, because it's a warning, lots of people will notice it before
their asking the FAQ again.
* Second, because all the information is on a single line and actually
contains the string "max_fsm_relations", it gives people a clue what
to do about it. (note that vacuum verbose uses similar phrases but
from the number of questions here, it must not be obvious)
* Third, I don't need the 'verbose' setting.
* And most importantly, our clients let us know about WARNINGs,
but not about INFOs or DETAILs in their log page; so it gives
us a chance to respond before their system drags to a halt.
If a patch like this could get into the standard distro, that'd be
awesome - just let me know what additional work is needed (I didn't
look at docs or internationalization yet). If not, I'd like to post
it here to patches just in case anyone else will benefit from the
same thing.
==================================================
% diff -u 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-23 14:58:50.638745744 -0800
@@ -704,6 +704,15 @@
/* Convert stats to actual number of page slots needed */
needed = (sumRequests + numRels) * CHUNKPAGES;
+
+ if (needed > MaxFSMPages)
+ ereport(WARNING,
+ (errmsg("max_fsm_pages(%d) is smaller than total pages needed(%.0f)",
+ MaxFSMPages, needed)));
+ if (numRels > MaxFSMRelations)
+ ereport(WARNING,
+ (errmsg("max_fsm_relations(%d) is smaller than the number of relations (%d)",
+ MaxFSMRelations, numRels)));
ereport(elevel,
(errmsg("free space map: %d relations, %d pages stored; %.0f total pages needed",
==================================================
Thoughts?
Ron
Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> + if (needed > MaxFSMPages)
> + ereport(WARNING,
> + (errmsg("max_fsm_pages(%d) is smaller than total pages needed(%.0f)",
> + MaxFSMPages, needed)));
An unconditional WARNING seems a bit strong to me for a case that is not
necessarily wrong. Depending on the needs of the installation, this
might be a perfectly acceptable situation --- for example if you have
lots of large read-mostly tables.
On the other side of the coin, the test could pass (ie no warning) in
situations where in fact MaxFSMPages is too small, because what we are
comparing it to is the number of pages requested for relations that are
being tracked. If MaxFSMRelations is too small then we can't really
tell whether MaxFSMPages is adequate.
> + if (numRels > MaxFSMRelations)
> + ereport(WARNING,
> + (errmsg("max_fsm_relations(%d) is smaller than the number of relations (%d)",
> + MaxFSMRelations, numRels)));
This part is just plain dead code, since it's not possible for numRels
to exceed MaxFSMRelations.
I think it might be useful to warn when numRels == MaxFSMRelations,
since if you don't have even one spare fsmrel slot then you probably
have too few (it's unlikely you got it on the nose). But I don't know
how to produce a warning about MaxFSMPages that's worth anything.
regards, tom lane
> I find this tiny (9-line) patch useful to help my clients know
> when FSM settings may need updating.
>
> Some of the more frequently asked questions here are in regards to FSM
> settings. One hint I've seen is to run "vacuum verbose;". At the end
> of thousands of lines of INFO and DETAIL messages vacuum verbose has 2
> separate lines with some numbers to compare ("total pages needed" and
> "FSM size...pages") that help indicate too low fsm settings.
>
>
> I've gotten into the habit of always installing the following patch
> (below) that automatically does this comparison for me, and if
> max_fsm_pages is too small, it logs a warning as shown here:
>
> patched=# vacuum;
> WARNING: max_fsm_pages(1601) is smaller than total pages needed(2832)
> VACUUM
I think this patch is great. I can never figure out how to set those
settings easily.
Chris
On Wed, 2005-02-23 at 19:31 -0500, Tom Lane wrote:
> Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> > + if (needed > MaxFSMPages)
> > + ereport(WARNING,
> > + (errmsg("max_fsm_pages(%d) is smaller than total pages needed(%.0f)",
> > + MaxFSMPages, needed)));
>
> An unconditional WARNING seems a bit strong to me for a case that is not
> necessarily wrong. Depending on the needs of the installation, this
> might be a perfectly acceptable situation --- for example if you have
> lots of large read-mostly tables.
The patch seems very useful to me. I had been thinking about doing
something like that myself.
VACUUM uses an INFO to provide the "total pages needed", so it should be
a simple matter to change the ereport to an INFO rather than WARNING as
well.
It would be great to have both lines of INFO, so that VACUUM would
produce output like this:
patched=# vacuum;
INFO: free space map: 77 relations, 470 pages stored
INFO: max_fsm_pages(1601) is smaller than total pages needed(2832)
DETAIL: Allocated FSM size: 100 relations + 1601 pages = 19 kB shared
memory.
VACUUM
...where the second info line was conditional...like this...
+ if (numRels == MaxFSMRelations)
+ ereport(WARNING,
+ (errmsg("max_fsm_relations(%d) may be set too low",
+ MaxFSMRelations)));
+ else
+ if (needed > MaxFSMPages)
+ ereport(INFO,
+ (errmsg("max_fsm_pages(%d) is smaller than total pages
needed(%.0f)",
+ MaxFSMPages, needed)));
ereport(elevel,
(errmsg("free space map: %d relations, %d pages stored;
%.0f total pages needed",
Which goes more towards Tom's gripes.
The manual could have a line added to explain that if max_fsm_relations
is set too low, then max_fsm_pages may also inadvertently be too low,
yet not be obvious that that is the case.
Best Regards, Simon Riggs
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
formax_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
============================================================
Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> Would the updated patch below address most of the concerns?
I preferred Simon's idea of not trying to produce a warning for pages
when we've detected relation overflow.
Making it a LOG rather than WARNING does address the issue of being
too much in-your-face for an uncertain condition, though.
regards, tom lane
On Thu, 24 Feb 2005, Tom Lane wrote: > I preferred Simon's idea of not trying to produce a warning for pages > when we've detected relation overflow. Sounds good. I'll make that update. Should the relation overflow be a WARNING or a LOG? It sounds like if you have that problem it's almost certainly a problem, right? > Making it a LOG rather than WARNING does address the issue of being > too much in-your-face for an uncertain condition, though. Great.
Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> Should the relation overflow be a WARNING or a LOG? It sounds like
> if you have that problem it's almost certainly a problem, right?
I'd go for making them both LOG, I think. More consistent.
regards, tom lane
On Thu, 24 Feb 2005, Ron Mayer wrote: > Should the relation overflow be a WARNING or a LOG? It sounds like > if you have that problem it's almost certainly a problem, right? And while I'm at it... what's the convention for INFOs vs LOGs? The "checkpoint...too frequent" seemed similar, and is a LOG. And do people think the HINT's I added add value or just noise?
On Thu, 24 Feb 2005, Tom Lane wrote:
> I'd go for making them both LOG, I think. More consistent.
Ok, here's another try :) With a couple more questions...
1. If I read Simon's email correctly, it implied that he wanted to see
the "free space map" message for a VACUUM even when VERBOSE is turned off.
I could just tweak it in PrintFreeSpaceMapStastics() as shown here...
but now elevel (which depended on VACUUM VERBOSE or not) is no longer
needed by PrintFreeSpaceMapStastics.
1a. Is that desired to always show this line as an INFO instead of
a DEBUG2 (which it currently is when VERBOSE is not selected)?
1b. Should I tweak vacuum.c (feels cleaner) or just freespace.c (minimal
changes).
2. If I read Simon's email correctly, it implied that he wanted to see
these new lines when you type VACUUM. This would suggest making
them INFOs. Making them INFOs would slightly contradict another
goal of wanting to see them in the LOG for automated log
grepping scripts to find, since that would require turning on INFOs
that I think commonly aren't logged. Also making them LOGs is
consistent with the checkpoint hint. I suppose they could be
made NOTICEs; but that isn't consistent with either.
diff -u 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 14:54:36.619566040 -0800
@@ -705,12 +705,25 @@
/* Convert stats to actual number of page slots needed */
needed = (sumRequests + numRels) * CHUNKPAGES;
- ereport(elevel,
+ ereport(INFO,
(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 (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
formax_fsm_pages may have been underestimated. ",numRels)));
+ else
+ 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)));
+
}
Tom Lane wrote: > Ron Mayer <rm_pg@cheapcomplexdevices.com> writes: > > Should the relation overflow be a WARNING or a LOG? It sounds like > > if you have that problem it's almost certainly a problem, right? > > I'd go for making them both LOG, I think. More consistent. Can we also update this wording: INFO: free space map: 52 relations, 61 pages stored; 848 total pages needed DETAIL: Allocated FSM size: 1000 relations + 20000 pages = 182 kB shared memory. The "pages needed" is confusing. In fact it is the total pages used or allocated. I looked in the code and got confused. It needs clarity. Right now it sounds like you need more when you don't. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Fri, 25 Feb 2005, Bruce Momjian wrote:
> Tom Lane wrote:
> > Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> > > Should the relation overflow be a WARNING or a LOG? ...
> > I'd go for making them both LOG, I think. More consistent.
>
> Can we also update this wording:
>
> INFO: free space map: 52 relations, 61 pages stored; 848 total pages needed
> DETAIL: Allocated FSM size: 1000 relations + 20000 pages = 182 kB shared memory.
>
> The "pages needed" is confusing. In fact it is the total pages used or
> allocated. I looked in the code and got confused. It needs clarity.
Any preference? To me, "allocated" has some risk of sounding like
it refers to the total free space map (memory allocated for fsm)
instead of just the used ones. "Allocated" is actually used for
that other meaning on the next line. I guess it's confusing there
too, so that line should be changed as well.
How about if I go for "used" in that first line; and simply remove the
word "Allocated" in the DETAIL line.
So instead of:
>
> INFO: free space map: 52 relations, 61 pages stored; 848 total pages needed
> DETAIL: Allocated FSM size: 1000 relations + 20000 pages = 182 kB shared memory.
>
it'll say
>
> INFO: free space map: 52 relations, 61 pages stored; 848 total pages used
> DETAIL: FSM size: 1000 relations + 20000 pages = 182 kB shared memory.
>
With those changes, the patch now looks like this...
======================================================================
% diff -u 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-25 16:45:26.773792440 -0800
@@ -705,12 +705,25 @@
/* 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",
+ ereport(INFO,
+ (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
numRels, storedPages, needed),
- errdetail("Allocated FSM size: %d relations + %d pages = %.0f kB shared memory.",
+ errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
MaxFSMRelations, MaxFSMPages,
(double) FreeSpaceShmemSize() / 1024.0)));
+
+ 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
formax_fsm_pages may have been underestimated. ",numRels)));
+ else
+ 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)));
+
}
/*
======================================================================
Getting closer?
On Fri, 2005-02-25 at 16:48 -0800, Ron Mayer wrote: > Getting closer? For me, yes. I agree with Bruce's comment on the use of the word "needed", and I think your change reads better now. The not-warnings seem a little wordy for me, but they happen when and how I would hope for. So, for me, it looks like a polish of final wording and commit. Best Regards, Simon Riggs
On Sun, 27 Feb 2005, Simon Riggs wrote:
> On Fri, 2005-02-25 at 16:48 -0800, Ron Mayer wrote:
> > Getting closer?
> For me, yes. [...]
> The not-warnings seem a little wordy for me, but they happen when and
> how I would hope for.
>
> So, for me, it looks like a polish of final wording and commit.
Thanks for the feedback. How about I replace the grammatically poor:
LOG: max_fsm_relations(%d) is equal than the number of relations vacuum checked (%d)",
HINT: You probably have more than %d relations. You should increase max_fsm_relations. Pages needed for
max_fsm_pages may have been underestimated.
with this:
LOG: max_fsm_relations(100) equals the number of relations checked
HINT: You have >= 100 relations. You should increase max_fsm_relations.
and replace this:
LOG: max_fsm_pages(%d) is smaller than the actual number of page slots needed(%.0f)",
HINT: You may want to increase max_fsm_pages to be larger than %.0f"
with the slightly smaller
LOG: the number of page slots needed (2832) exceeds max_fsm_pages (1601)
HINT: You may want to increase max_fsm_pages to a value over 2832.
These updated messages would fit on an 80-column display if the numbers
aren't too big. Here's 80 characters for a quick reference.
01234567890123456789012345678901234567890123456789012345678901234567890123456789
The "pages needed...underestimate" in the first message was no longer
useful anyway; since it's no longer logging fsm_pages stuff when the
max_fsm_relations condition occurred anyway
Ron
The patch now looks like:
================================================================================
% diff -u 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-27 11:54:39.776546200 -0800
@@ -705,12 +705,25 @@
/* 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",
+ ereport(INFO,
+ (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
numRels, storedPages, needed),
- errdetail("Allocated FSM size: %d relations + %d pages = %.0f kB shared memory.",
+ errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
MaxFSMRelations, MaxFSMPages,
(double) FreeSpaceShmemSize() / 1024.0)));
+
+ if (numRels == MaxFSMRelations)
+ ereport(LOG,
+ (errmsg("max_fsm_relations(%d) equals the number of relations checked",
+ MaxFSMRelations),
+ errhint("You have >= %d relations. You should increase max_fsm_relations.",numRels)));
+ else
+ if (needed > MaxFSMPages)
+ ereport(LOG,
+ (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)",
+ needed,MaxFSMPages),
+ errhint("You may want to increase max_fsm_pages to a value over %.0f.",needed)));
+
}
/*
%
================================================================================
I have applied your patch with minor modifications. Applied version
attached.
I think the "pages" message:
INFO: free space map: 44 relations, 28 pages stored; 704 total pages used
DETAIL: FSM size: 1000 relations + 20000 pages = 182 kB shared memory.
should remain DEBUG2 for non-VERBOSE, and INFO for VERBOSE. The
information is pretty complex and probably of little interest to a
typical vacuum user. In fact, the new messages make the information
even less important because problems are now flagged.
I adjusted your output levels for the new messages. I realize the
"checkpoint warning" is a LOG message, but it has to be because there is
no active session when a checkpoint is being run. In the case of
VACUUM, there is an active session so I think the messages should be
sent to that session. Sending them to the logs and not to the user
seems unusual because they are the ones who asked for the VACUUM. I
realize they might not be able to change the server settings.
These new messages:
NOTICE: max_fsm_relations(1000) equals the number of relations checked
HINT: You have >= 44 relations.
Consider increasing the configuration parameter "max_fsm_relations".
NOTICE: the number of page slots needed (704) exceeds max_fsm_pages (20000)
HINT: Consider increasing the configuration parameter "max_fsm_relations"
to a value over 704.
VACUUM
should be NOTICE. NOTICE is for unusual events that are not warnings,
and that fits these cases. If the administrator wants those in the
logs, he can set log_min_messages to NOTICE.
I also adjusted your output strings to more closely match our checkpoint
warning message.
Another idea would be to send the output to both the client and the logs
by default.
---------------------------------------------------------------------------
Ron Mayer wrote:
> On Sun, 27 Feb 2005, Simon Riggs wrote:
> > On Fri, 2005-02-25 at 16:48 -0800, Ron Mayer wrote:
> > > Getting closer?
> > For me, yes. [...]
> > The not-warnings seem a little wordy for me, but they happen when and
> > how I would hope for.
> >
> > So, for me, it looks like a polish of final wording and commit.
>
> Thanks for the feedback. How about I replace the grammatically poor:
>
> LOG: max_fsm_relations(%d) is equal than the number of relations vacuum checked (%d)",
> HINT: You probably have more than %d relations. You should increase max_fsm_relations. Pages needed for
> max_fsm_pages may have been underestimated.
>
> with this:
>
> LOG: max_fsm_relations(100) equals the number of relations checked
> HINT: You have >= 100 relations. You should increase max_fsm_relations.
>
>
> and replace this:
>
> LOG: max_fsm_pages(%d) is smaller than the actual number of page slots needed(%.0f)",
> HINT: You may want to increase max_fsm_pages to be larger than %.0f"
>
> with the slightly smaller
>
> LOG: the number of page slots needed (2832) exceeds max_fsm_pages (1601)
> HINT: You may want to increase max_fsm_pages to a value over 2832.
>
>
> These updated messages would fit on an 80-column display if the numbers
> aren't too big. Here's 80 characters for a quick reference.
> 01234567890123456789012345678901234567890123456789012345678901234567890123456789
> The "pages needed...underestimate" in the first message was no longer
> useful anyway; since it's no longer logging fsm_pages stuff when the
> max_fsm_relations condition occurred anyway
>
> Ron
>
> The patch now looks like:
>
> ================================================================================
> % diff -u 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-27 11:54:39.776546200 -0800
> @@ -705,12 +705,25 @@
> /* 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",
> + ereport(INFO,
> + (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
> numRels, storedPages, needed),
> - errdetail("Allocated FSM size: %d relations + %d pages = %.0f kB shared memory.",
> + errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
> MaxFSMRelations, MaxFSMPages,
> (double) FreeSpaceShmemSize() / 1024.0)));
> +
> + if (numRels == MaxFSMRelations)
> + ereport(LOG,
> + (errmsg("max_fsm_relations(%d) equals the number of relations checked",
> + MaxFSMRelations),
> + errhint("You have >= %d relations. You should increase max_fsm_relations.",numRels)));
> + else
> + if (needed > MaxFSMPages)
> + ereport(LOG,
> + (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)",
> + needed,MaxFSMPages),
> + errhint("You may want to increase max_fsm_pages to a value over %.0f.",needed)));
> +
> }
>
> /*
> %
> ================================================================================
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/backend/storage/freespace/freespace.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/freespace/freespace.c,v
retrieving revision 1.37
diff -c -c -r1.37 freespace.c
*** src/backend/storage/freespace/freespace.c 31 Dec 2004 22:00:54 -0000 1.37
--- src/backend/storage/freespace/freespace.c 12 Mar 2005 05:05:47 -0000
***************
*** 706,716 ****
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)));
}
/*
--- 706,730 ----
needed = (sumRequests + numRels) * CHUNKPAGES;
ereport(elevel,
! (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
numRels, storedPages, needed),
! errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
MaxFSMRelations, MaxFSMPages,
(double) FreeSpaceShmemSize() / 1024.0)));
+
+ if (numRels == MaxFSMRelations)
+ ereport(NOTICE,
+ (errmsg("max_fsm_relations(%d) equals the number of relations checked",
+ MaxFSMRelations),
+ errhint("You have >= %d relations.\n"
+ "Consider increasing the configuration parameter \"max_fsm_relations\".",
+ numRels)));
+ else if (needed > MaxFSMPages)
+ ereport(NOTICE,
+ (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)",
+ needed,MaxFSMPages),
+ errhint("Consider increasing the configuration parameter \"max_fsm_relations\"\n"
+ "to a value over %.0f.", needed)));
}
/*
Well, I was really hoping something would end up in the log file.
The situation is that our clients - sometimes not that computer
savvy - go perhaps a year without us being involved (unless
the log monitoring scripts show something abnormal; or if the
system breaks).
The primary motivation for tweaking this file in the first place
was so that the log file would catch the situation where their
database outgrows the FSM settings before it causes a problem.
What about at least sending the output to a log file
if VERBOSE or some GUC variable is set?
Ron
Bruce Momjian wrote:
> I have applied your patch with minor modifications. Applied version
> attached.
>
> I think the "pages" message:
>
> INFO: free space map: 44 relations, 28 pages stored; 704 total pages used
> DETAIL: FSM size: 1000 relations + 20000 pages = 182 kB shared memory.
>
> should remain DEBUG2 for non-VERBOSE, and INFO for VERBOSE. The
> information is pretty complex and probably of little interest to a
> typical vacuum user. In fact, the new messages make the information
> even less important because problems are now flagged.
>
> I adjusted your output levels for the new messages. I realize the
> "checkpoint warning" is a LOG message, but it has to be because there is
> no active session when a checkpoint is being run. In the case of
> VACUUM, there is an active session so I think the messages should be
> sent to that session. Sending them to the logs and not to the user
> seems unusual because they are the ones who asked for the VACUUM. I
> realize they might not be able to change the server settings.
>
> These new messages:
>
> NOTICE: max_fsm_relations(1000) equals the number of relations checked
> HINT: You have >= 44 relations.
> Consider increasing the configuration parameter "max_fsm_relations".
> NOTICE: the number of page slots needed (704) exceeds max_fsm_pages (20000)
> HINT: Consider increasing the configuration parameter "max_fsm_relations"
> to a value over 704.
> VACUUM
>
> should be NOTICE. NOTICE is for unusual events that are not warnings,
> and that fits these cases. If the administrator wants those in the
> logs, he can set log_min_messages to NOTICE.
>
> I also adjusted your output strings to more closely match our checkpoint
> warning message.
>
> Another idea would be to send the output to both the client and the logs
> by default.
>
> ---------------------------------------------------------------------------
>
> Ron Mayer wrote:
>
>>On Sun, 27 Feb 2005, Simon Riggs wrote:
>>
>>>On Fri, 2005-02-25 at 16:48 -0800, Ron Mayer wrote:
>>>
>>>>Getting closer?
>>>
>>>For me, yes. [...]
>>>The not-warnings seem a little wordy for me, but they happen when and
>>>how I would hope for.
>>>
>>>So, for me, it looks like a polish of final wording and commit.
>>
>>Thanks for the feedback. How about I replace the grammatically poor:
>>
>> LOG: max_fsm_relations(%d) is equal than the number of relations vacuum checked (%d)",
>> HINT: You probably have more than %d relations. You should increase max_fsm_relations. Pages needed for
>>max_fsm_pages may have been underestimated.
>>
>>with this:
>>
>> LOG: max_fsm_relations(100) equals the number of relations checked
>> HINT: You have >= 100 relations. You should increase max_fsm_relations.
>>
>>
>>and replace this:
>>
>> LOG: max_fsm_pages(%d) is smaller than the actual number of page slots needed(%.0f)",
>> HINT: You may want to increase max_fsm_pages to be larger than %.0f"
>>
>>with the slightly smaller
>>
>> LOG: the number of page slots needed (2832) exceeds max_fsm_pages (1601)
>> HINT: You may want to increase max_fsm_pages to a value over 2832.
>>
>>
>>These updated messages would fit on an 80-column display if the numbers
>>aren't too big. Here's 80 characters for a quick reference.
>> 01234567890123456789012345678901234567890123456789012345678901234567890123456789
>>The "pages needed...underestimate" in the first message was no longer
>>useful anyway; since it's no longer logging fsm_pages stuff when the
>>max_fsm_relations condition occurred anyway
>>
>> Ron
>>
>>The patch now looks like:
>>
>>================================================================================
>>% diff -u 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-27 11:54:39.776546200 -0800
>>@@ -705,12 +705,25 @@
>> /* 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",
>>+ ereport(INFO,
>>+ (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
>> numRels, storedPages, needed),
>>- errdetail("Allocated FSM size: %d relations + %d pages = %.0f kB shared memory.",
>>+ errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
>> MaxFSMRelations, MaxFSMPages,
>> (double) FreeSpaceShmemSize() / 1024.0)));
>>+
>>+ if (numRels == MaxFSMRelations)
>>+ ereport(LOG,
>>+ (errmsg("max_fsm_relations(%d) equals the number of relations checked",
>>+ MaxFSMRelations),
>>+ errhint("You have >= %d relations. You should increase max_fsm_relations.",numRels)));
>>+ else
>>+ if (needed > MaxFSMPages)
>>+ ereport(LOG,
>>+ (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)",
>>+ needed,MaxFSMPages),
>>+ errhint("You may want to increase max_fsm_pages to a value over %.0f.",needed)));
>>+
>> }
>>
>> /*
>>%
>>================================================================================
>>
>
>
>
> ------------------------------------------------------------------------
>
> Index: src/backend/storage/freespace/freespace.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/storage/freespace/freespace.c,v
> retrieving revision 1.37
> diff -c -c -r1.37 freespace.c
> *** src/backend/storage/freespace/freespace.c 31 Dec 2004 22:00:54 -0000 1.37
> --- src/backend/storage/freespace/freespace.c 12 Mar 2005 05:05:47 -0000
> ***************
> *** 706,716 ****
> 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)));
> }
>
> /*
> --- 706,730 ----
> needed = (sumRequests + numRels) * CHUNKPAGES;
>
> ereport(elevel,
> ! (errmsg("free space map: %d relations, %d pages stored; %.0f total pages used",
> numRels, storedPages, needed),
> ! errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.",
> MaxFSMRelations, MaxFSMPages,
> (double) FreeSpaceShmemSize() / 1024.0)));
> +
> + if (numRels == MaxFSMRelations)
> + ereport(NOTICE,
> + (errmsg("max_fsm_relations(%d) equals the number of relations checked",
> + MaxFSMRelations),
> + errhint("You have >= %d relations.\n"
> + "Consider increasing the configuration parameter \"max_fsm_relations\".",
> + numRels)));
> + else if (needed > MaxFSMPages)
> + ereport(NOTICE,
> + (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)",
> + needed,MaxFSMPages),
> + errhint("Consider increasing the configuration parameter \"max_fsm_relations\"\n"
> + "to a value over %.0f.", needed)));
> }
>
> /*
Ron Mayer wrote: > Well, I was really hoping something would end up in the log file. > > The situation is that our clients - sometimes not that computer > savvy - go perhaps a year without us being involved (unless > the log monitoring scripts show something abnormal; or if the > system breaks). > > The primary motivation for tweaking this file in the first place > was so that the log file would catch the situation where their > database outgrows the FSM settings before it causes a problem. > > What about at least sending the output to a log file > if VERBOSE or some GUC variable is set? You didn't like server_min_messages = 'notify'? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian wrote:
>
> You didn't like server_min_messages = 'notify'?
I merely don't have a feeling for how much additional stuff
verbose would be putting in the log files.
If it's a good practice for production systems to be logging
NOTIFY's I'm happy with the change.
My reasoning why I thought the log file was more useful was
that only an admin with access to the log files could really
do anything about the message anyway.
Also since the message happing occasionally is probably OK,
yet if it happens a lot it's more likely worth looking
into - I think the historical record of when it happened
is more interesting than a one-time occurrence which is
all you seen in the active session.
Ron
PS: I'm fine either way; and perhaps it's a good idea
for me to be logging NOTIFY's anyway -- I just thought
I'd explain my reasoning above. I'm sure you guys know
a lot more than me what best practices would be.
Ron Mayer wrote:
> Bruce Momjian wrote:
> >
> > You didn't like server_min_messages = 'notify'?
>
> I merely don't have a feeling for how much additional stuff
> verbose would be putting in the log files.
You will probably see the creation of indexes and sequences, like this:
test=> create table test(x int primary key);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
"test_pkey" for table "test"
> If it's a good practice for production systems to be logging
> NOTIFY's I'm happy with the change.
Not really. The FSM message has a lot more interest than the other
NOTIFY messages.
> My reasoning why I thought the log file was more useful was
> that only an admin with access to the log files could really
> do anything about the message anyway.
The log file is useful, but I think showing the VACUUM user is _more_
useful than the log file.
> Also since the message happing occasionally is probably OK,
> yet if it happens a lot it's more likely worth looking
> into - I think the historical record of when it happened
> is more interesting than a one-time occurrence which is
> all you seen in the active session.
Seems it could be made to be both client and log, but I can't think of
any case were we do that now, and am unsure it is a good idea to add it
just for this.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Ron Mayer wrote:
>> My reasoning why I thought the log file was more useful was
>> that only an admin with access to the log files could really
>> do anything about the message anyway.
> The log file is useful, but I think showing the VACUUM user is _more_
> useful than the log file.
I think that reasoning is fundamentally unsound, because (a) a lot of
people already do vacuuming via a cron job or autovacuum, and (b)
autovacuum is definitely the wave of the future. So it's foolish
to design this messaging around the assumption that there will be
a human attentive to the on-line output from VACUUM. We should be
ensuring that the message gets into the postmaster log --- whether
it gets sent to the client is secondary.
regards, tom lane
On Mon, 2005-03-14 at 01:40 -0500, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Ron Mayer wrote: > >> My reasoning why I thought the log file was more useful was > >> that only an admin with access to the log files could really > >> do anything about the message anyway. > > > The log file is useful, but I think showing the VACUUM user is _more_ > > useful than the log file. > > I think that reasoning is fundamentally unsound, because (a) a lot of > people already do vacuuming via a cron job or autovacuum, and (b) > autovacuum is definitely the wave of the future. So it's foolish > to design this messaging around the assumption that there will be > a human attentive to the on-line output from VACUUM. We should be > ensuring that the message gets into the postmaster log --- whether > it gets sent to the client is secondary. Personally, I prefer the postmaster log as the place for this. However, whilst vacuum exists as a separate command, there will be an argument to return a message back to the person running it; we cannot assume that people would be inattentive. Possibly the deciding factor should be whether autovacuum makes it fully into becoming a special backend anytime soon, since in that case only the log would remain as an option for reporting this message, in that case. Can we have both? Best Regards, Simon Riggs
Simon Riggs wrote: > On Mon, 2005-03-14 at 01:40 -0500, Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > Ron Mayer wrote: > > >> My reasoning why I thought the log file was more useful was > > >> that only an admin with access to the log files could really > > >> do anything about the message anyway. > > > > > The log file is useful, but I think showing the VACUUM user is _more_ > > > useful than the log file. > > > > I think that reasoning is fundamentally unsound, because (a) a lot of > > people already do vacuuming via a cron job or autovacuum, and (b) > > autovacuum is definitely the wave of the future. So it's foolish > > to design this messaging around the assumption that there will be > > a human attentive to the on-line output from VACUUM. We should be > > ensuring that the message gets into the postmaster log --- whether > > it gets sent to the client is secondary. > > Personally, I prefer the postmaster log as the place for this. > > However, whilst vacuum exists as a separate command, there will be an > argument to return a message back to the person running it; we cannot > assume that people would be inattentive. > > Possibly the deciding factor should be whether autovacuum makes it fully > into becoming a special backend anytime soon, since in that case only > the log would remain as an option for reporting this message, in that > case. > > Can we have both? Sure. It is very easy and in fact looks even cleaner than the original code because now the optional stuff is in its own function. Patch attached and applied. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: src/backend/storage/freespace/freespace.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/storage/freespace/freespace.c,v retrieving revision 1.38 diff -c -c -r1.38 freespace.c *** src/backend/storage/freespace/freespace.c 12 Mar 2005 05:21:52 -0000 1.38 --- src/backend/storage/freespace/freespace.c 14 Mar 2005 20:04:00 -0000 *************** *** 221,226 **** --- 221,228 ---- * FSMHeader->relHash */ + static void CheckFreeSpaceMapStatistics(int elevel, int numRels, + double needed); static FSMRelation *lookup_fsm_rel(RelFileNode *rel); static FSMRelation *create_fsm_rel(RelFileNode *rel); static void delete_fsm_rel(FSMRelation *fsmrel); *************** *** 711,726 **** errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.", MaxFSMRelations, MaxFSMPages, (double) FreeSpaceShmemSize() / 1024.0))); ! ! if (numRels == MaxFSMRelations) ! ereport(NOTICE, (errmsg("max_fsm_relations(%d) equals the number of relations checked", MaxFSMRelations), errhint("You have >= %d relations.\n" "Consider increasing the configuration parameter \"max_fsm_relations\".", numRels))); else if (needed > MaxFSMPages) ! ereport(NOTICE, (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)", needed,MaxFSMPages), errhint("Consider increasing the configuration parameter \"max_fsm_relations\"\n" --- 713,736 ---- errdetail("FSM size: %d relations + %d pages = %.0f kB shared memory.", MaxFSMRelations, MaxFSMPages, (double) FreeSpaceShmemSize() / 1024.0))); ! ! CheckFreeSpaceMapStatistics(NOTICE, numRels, needed); ! /* Print to server logs too because is deals with a config variable. */ ! CheckFreeSpaceMapStatistics(LOG, numRels, needed); ! } ! ! static void ! CheckFreeSpaceMapStatistics(int elevel, int numRels, double needed) ! { ! if (numRels == MaxFSMRelations) ! ereport(elevel, (errmsg("max_fsm_relations(%d) equals the number of relations checked", MaxFSMRelations), errhint("You have >= %d relations.\n" "Consider increasing the configuration parameter \"max_fsm_relations\".", numRels))); else if (needed > MaxFSMPages) ! ereport(elevel, (errmsg("the number of page slots needed (%.0f) exceeds max_fsm_pages (%d)", needed,MaxFSMPages), errhint("Consider increasing the configuration parameter \"max_fsm_relations\"\n"