List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:January 19 2011 8:33pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)
View as plain text  
On 1/19/11 6:14 PM, Guilhem Bichot wrote:
> Hello,
>
> Davi Arnaut a écrit, Le 19.01.2011 20:08:
>> On 1/19/11 4:58 PM, Guilhem Bichot wrote:
>>> #At
>>> file:///home/mysql_src/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800/
>>> based on revid:guilhem.bichot@stripped
>>>
>>> 3260 Guilhem Bichot 2011-01-19
>>> todo question
>>>
>>> modified:
>>> WL4800_TODO.txt
>>> === modified file 'WL4800_TODO.txt'
>>> --- a/WL4800_TODO.txt 2011-01-17 20:41:34 +0000
>>> +++ b/WL4800_TODO.txt 2011-01-19 18:58:53 +0000
>>> @@ -44,3 +44,7 @@ Make --opt-trace-protocol dump traces to
>>> can run with it without failing all tests.
>>>
>>> Update copyright header of all changed files to reflect changed in 2011
>>> +
>>> +should the debug binary really assert(0) if json syntax error? is it a
>>> +good idea at the customer's? On the other hand, how to make sure a
>>> +developer notices a syntax error when running tests?
>>>
>>
>> Perhaps it is time for us to have a DEBUG_WARN?
>
> What would this macro do?

Emit a warning! :-)

> The context is that if a customer/support is troubleshooting a
> problematic query, using the optimizer trace and a debug binary, it may
> happen that she/he hits a case where the trace is not JSON-compliant

I assumed that since you mentioned debug binaries and assert(0) a 
customer/support using it is the most rare case and actually a bit 
dangerous. The main and ultimate audience of debug binaries are developers.

> (we, optimizer team, cannot guarantee that we have studied *all* code
> paths which can lead to generation of a trace; there are many more than
> what is exercised by the full mtr testsuite).
> In that case, for the customer, the invalid-syntax trace is still worth
> looking at, informative, and a DBUG_ASSERT(0) is then a pure annoyance.

Do not worry about customers and debug binaries. We should be asking, is 
this useful for developers? In any case, even if a customer is running a 
debug binary, the target of the said test would be to send information 
to a developer. In this case, a DBUG_ASSERT(0) would be useful to the 
developer?

> On the other hand, when I, developer, am modifying the optimizer and
> running the testsuite, I certainly want to be told if my changes lead to
> an invalid trace, and for this, DBUG_ASSERT(0) is perfect (it makes the
> test fail).

I think you have the answer you are looking for :)

> Maybe we could do a sql_print_warning() instead of DBUG_ASSERT(0); mtr
> warns when it sees an unexpected warning, it marks the test as failed,
> prints the warning on stderr, and returns 1 as exit code. I guess this
> would be good enough?

I think it depends on the gravity of the occurrence. For example, if the 
unmet condition can cause significant trouble (i.e. crash) in a 
production binary, it warrants a assertion. If not, emitting a warning 
would be more appropriate.

Drawing a comparison, the Linux kernel has a pair of macros for those 
two scenarios, WARN_ON() and BUG_ON(). The latter stops the machine by 
generating an intentional trap, while the latter just prints a detailed 
message to system log. BUG_ON() is used when the error is unrecoverable 
and fatal, while WARN_ON() is used for recoverable errors/bugs that 
should be addressed somehow.

Since MTR detects warnings written to the error log, this logic could be 
applied for MySQL as well. Whether to warn or to panic would depend on 
the gravity of the failed assumption. Perhaps, also on the amount of 
information needed to investigate the failed condition, as generating a 
trap will give us a nice core file.

Regards,

Davi
Thread
bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260) Guilhem Bichot19 Jan
Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Davi Arnaut19 Jan
  • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Guilhem Bichot19 Jan
    • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Davi Arnaut19 Jan
      • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Guilhem Bichot20 Jan
    • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Bjorn Munch19 Jan
      • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem.bichot:3260)Davi Arnaut19 Jan