Re: [PATCHES] A way to let Vacuum warn if FSM settings are low. [final?]

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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073