Re: [PATCHES] A way to let Vacuum warn if FSM settings are low. - Mailing list pgsql-general

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