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
