Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3192) Bug#42851

[Davi Arnaut]

Hi All,

I think you guys need to stop for a moment, take a deep breath and 
reflect about what problem needs to be solved. The problem is that a 
user might trigger messages to be written to the error log, which is a 
concern since disk space for the error log is limited. The concern is 
about messages written to the error log.. any kind (be it a warning, a 
error or a note). This has nothing to do with push_warning nor it is 
exclusive to replication code -- the case reported on the bug is just 
one of the various that a user can trigger.

Please, let's not complicate this further.

On 11/26/09 10:02 AM, Andrei Elkin wrote:
> ZhenXing, DaoGang, hello.
>
> Thanks to talking to Serge today we have come to a conclusion that the bug
> needs merely replacing sql_print_warning() with push_warning() in pieces of our
> replication code that warns on unsafety to binlog.
>
> *Another* worklog can make the push_warning-ed warning be present in the error log.
> That can be accomplished e.g by means of improving/extending --log-warnings.
> Consider earlier suggested method in the mail:
>
>     Date: Wed, 08 Jul 2009 17:05:00 +0300
>     From: Andrei Elkin<Andrei.Elkin@stripped>
>     Subject: Re: BUG#42851, BUG#39934 and suppress_warnings proposal: bringing back
>     discussion.
>
>     Currently there is the only tool to control error-logging with
>     --log-warnings which is supposed to deal with levels
>     (look at the definition given by PeterG in the attached reply)
>     rather than a particular error code.
>
>     It's difficult to say what was the original intention of the option.
>     Still docs lines make me think it is about switching on/off error-logging
>     for whole levels.
>     However, it works strongly different.
>
>     This mail carries two proposals.
>
>     First,
>
>     to refine --log-warnings to correspond the spirit of the definition
>     to make it possible the switching. E.g the following dbug-like
>     syntax can be offered (per Luis):
>
>     --log-warnings=-error,+warning,+note
>
>     to mean to disable the error level and enable the warning and the note
>     level\footnote{to read to disable error-logging for ...}.
>
>     Second,
>
>     either to elaborate on --log-warning to supply a particular error codes
>
>     --log-warnings=-error:4711,1001,+note:1001
>
>     or to add a new separate option
>
>     --suppress-warnings=-error:4711,1001,+note:1001
>
>     meaning the same thing. In this example to exclude error-logging
>     the error 4711 and to include the note 1001 for error-logging.
>
>     If the dbug-like +/- syntax looks appropriate, we shall discuss other things
>     like what should be defaults, repeating an argument in controversal way (like
>     +option , -option) etc.
>
>     Could you please sent back your opinion about this initiative.
>
>     cheers,
>
>     Andrei
>
>
>     ============== attachment ============================
>
>     Peter Gulutzan<Peter.Gulutzan@stripped>  writes:
>
>     >  Hi Andrei,
>     >
>     >  Andrei Elkin wrote:
>     >  <cut>
>     >>  --log-warnings
>     >>
>     >>  that has currently rather vague definition claiming that it deals with
>     >>  certain "levels" (i wonder where those are defined).
>     >>
>     >  <cut>
>     >
>     >
>     >  The manual page "|SHOW WARNINGS| Syntax"
>     >
>     >  http://dev.mysql.com/doc/refman/6.0/en/show-warnings.html
>     >  has a phrase "Note-level warnings", and SHOW WARNINGS can
>     >  display "Level: Note" or "Level: Warning" or "Level: Error".
>     >  So there's no definition, but there's a hint we have three
>     >  levels.
>     >
>     >  Mark Leith once suggested that for the error log we should
>     >  suppress notes (Bug#38066); that was rejected.Possibly
>     >  giving trivial warnings a special SQLSTATE value is better
>     >  than trying to enhance a non-standard "notes" level. Also
>     >  DECLARE ... HANDLER doesn't know what to do with "levels"
>     >  other than exception, warning, or not found.Notes are trouble.
>
> Notice, that `+' syntax is really opposite to the Unix's duplicate removal (that
> David suggested in
> the quoted mail's thread).
>
> cheers,
>
> Andrei
>
>
>     Andrei Elkin wrote:
>     >  After sleeping, and sleeping on this matter I think we would agree on
>     >
>     >  1. Rating for the error log  should be done per session
>
>     Bug#42851 is about limiting **warnings** written into error log, error
>     messages will not be affected by this. So I think it's not necessary
>     that we need to make sure that every session got a chance to output it's
>     warnings. I think a global limitation over all warnings generated by the
>     server is good enough.
>
>     >  2. Rating should target at duplicate messages
>
>     Remove duplicated messages can reduce messages written into the error
>     log, but this can not solve the problem of bug#42851, we still need a
>     way to limit the total numbers of warnings going into error log within a
>     interval.
>
>     And I think the mechanism to remove duplicated messages should be done
>     for all kinds of messages written into error log, not just warnings.
>
>     >
>     >  Per implementation, I still think the numeric id, not a checksum is the key
> that should be associated
>     >  with the number of hits. That is because many of the messages have variable
> body.
>     >  Expiration logics can be borrowed from the submitted patch.
>     >
>     >  Technically  rate_limit_impl() could be extended with maintaining a private
> to
>     >  THD of the session the-mess-id-number-of-hits hash.
>     >
>     >  cheers,
>     >
>     >  Andrei
>     >
>     >
>     >
>     >
>     >     Dao Gang, Davi, Serge, hello.
>     >
>     >        >
>     >        >  The patch introduces the global limit whereas a limit per
> session might be a better
>     >        >  solution.
>     >        >  The global limitting has a drawback that if for instance two
> connectons try logging
>     >        >  the limit value can be hit by the first-and-only error message
> of the 2nd session.
>     >        >  Therefore the error-log observer would miss that 2nd session
> error message.
>     >        >
>     >        >  Was that really disscussed before?
>     >
>     >        Yes. We have discussed it. The 'a limit per session' solution will
> make the
>     >        volume of messages become uncontrollable, because the number of
> session is
>     >        inconstant.
>     >
>     >     But it's still limitted (let's call that C_max). Hence, if your two
> parameters applied in the session scope
>     >
>     >     --session-log-warnings-ratelimit-interval = 10
>     >     --session-log-warnings-ratelimit-burst = 5
>     >
>     >     thier accumulative effect would be roughly the same as
>     >
>     >     --server-log-warnings-ratelimit-interval = 10
>     >     --server-log-warnings-ratelimit-burst = 5 * C_max
>     >
>     >     but with the obvious advantage in that all sessions will contribute to
> the log.
>     >     Another advantage is in that the sessions can be discriminated on the
> basis of the parameters.
>     >
>     >     The above approach does not cover a concern that all messages needs
> presenting in the log, at least one time.
>     >     If we set up our goal that way then discussion that Sergei and Davi held
> on #dev today
>     >     could guide us to the solution.
>     >     Let me note here, that checksumming of the message can be replaced by
>     >     the warning numeric id which is cheap computatioan-wise but would force
>     >     us to enumerate all warnings that exist without ids.