Thread

1. simplifying emode_for_corrupt_record

   Robert Haas <robertmhaas@gmail.com> — 2010-06-25T21:07:33Z

   On Mon, Jun 14, 2010 at 10:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   > Bruce Momjian <bruce@momjian.us> writes:
   >> Magnus Hagander wrote:
   >>> It means that we can't prevent people from configuring their tools to
   >>> ignore important warning. We can't prevent them rom ignoring ERROR or
   >>> FATAL either...
   >
   >> My point is that most tools are going to look at the tag first to
   >> determine the severity of the message, and might even have
   >> log_min_messages set to ignore warnings.
   >
   > Why is this discussion based on the idea that we have to cater to
   > incorrectly written log-filtering apps?
   >
   > The correct log level for this message is LOG.  End of discussion.
   
   I spend a little bit of time analyzing this today and it appears to me
   that all of the calls to emode_for_corrupt_record() arrive via
   ReadRecord(), which itself takes an emode argument that is always
   passed by the caller as either LOG or PANIC.  Therefore, the effect of
   the first "if" test in emode_for_corrupt_record() is to reduce the
   logging level of messages coming from SR or the archive from LOG to
   WARNING.  (WARNING would be higher in an interactive session, but not
   here, per Tom's point.)  This seems clearly a bad idea, so I propose
   to rip it out, which simplifies this function considerably.  Proposed
   patch attached.  I wasn't totally sure what to do about the comments.
   
   Assuming this change makes sense, there is still the question of
   bounding the number of retries and eventually falling down hard, but
   that's really a separate issue and I'll write a separate email about
   it when I get my thoughts together.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company
   

2. Re: simplifying emode_for_corrupt_record

   Robert Haas <robertmhaas@gmail.com> — 2010-06-28T19:42:52Z

   On Fri, Jun 25, 2010 at 5:07 PM, Robert Haas <robertmhaas@gmail.com> wrote:
   > I spend a little bit of time analyzing this today and it appears to me
   > that all of the calls to emode_for_corrupt_record() arrive via
   > ReadRecord(), which itself takes an emode argument that is always
   > passed by the caller as either LOG or PANIC.  Therefore, the effect of
   > the first "if" test in emode_for_corrupt_record() is to reduce the
   > logging level of messages coming from SR or the archive from LOG to
   > WARNING.  (WARNING would be higher in an interactive session, but not
   > here, per Tom's point.)  This seems clearly a bad idea, so I propose
   > to rip it out, which simplifies this function considerably.  Proposed
   > patch attached.
   
   Since this appears to be non-controversial, I'm going to go ahead and commit it.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company