Davi Arnaut a écrit, Le 19.01.2011 21:33:
> On 1/19/11 6:14 PM, Guilhem Bichot wrote:
>> Davi Arnaut a écrit, Le 19.01.2011 20:08:
>>> On 1/19/11 4:58 PM, Guilhem Bichot wrote:
>>>> based on
>>>> 3260 Guilhem Bichot 2011-01-19
>>>> todo question
>>>> === 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! :-)
I guess it would call sql_print_warning() then. fprintf(stderr,
"Warning") is maybe not guaranteed to reach the log in all platforms and
>> 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.
I used to think so, when recently I got a support case where the
customer had installed the debug binary, to inspect a crash in the
Optimizer, in coordination with Support.
That's why I was wondering whether it was really a good idea that a
slightly incorrect optimizer trace would cause suicide of the server,
thus preventing the customer from reading this trace, which may have
been helpful for troubleshooting the optimizer's problem.
>> (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
DBUG_ASSERT(0) would tell us that the trace is not JSON-compliant. Which
gives us a bug to fix (we want traces to always be JSON-compliant). On
the other hand, for the customer's case, the most urgent task is not
fixing this bug but fixing the first reported optimizer problem which
led to filing a Support incident (the bad query plan, the crash in
production binaries, etc).
>> 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.
In a production binary, if the trace is not JSON-compliant, it's not
fatal, we don't generate a crash (we just note it somewhere in the trace
as an indication).
So it's not a severe issue, it does not mandate a crash.
> 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.
Right, a non JSON-compliant trace is something to address for us but not
> 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.
If I replace DBUG_ASSERT(0) with a warning, and an MTR tests shows this
warning, I can restart the test with --gdb and put a breakpoint in
sql_print_warning(). If I am in the capacity of restarting the test
(which may not be easy, if in pb2).
I need to think about all this :-)
Thanks for the ideas!