On Tue, Jul 03, 2007 at 04:42:02PM +0300, Georgi Kodinov wrote:
> Guilhem,
>
> On 03.07.2007, at 16:37, Guilhem Bichot wrote:
>
> >Hello,
> >
> >On Tue, Jul 03, 2007 at 10:36:40AM +0300, kgeorge@stripped wrote:
> >>ChangeSet@stripped, 2007-07-03 10:36:37+03:00, gkodinov@stripped
> >>+1 -0
> >> Bug #28983: 'reset master' in multiple threads and innodb tables
> >> asserts debug binary
> >>
> >> We can't reliably check if the binary log is opened without
> >> acquiring its mutex.
> >> Fixed by removing this check.
> >>
> >>
>
> <cut>
>
> >Are you sure that is_open()-without-mutex is really the cause of the
> >assertions observed by the bug reports? They were assertions in
> >binlog_close_connection() and binlog_commit(), which test also
> >contents of trans_log and of thd->options. In "DBUG_ASSERT(A && B)",
> >is it A or B which is observed false?
>
> The problem is fully reproducible using the excellent test program
> attached to the bug. I've been able to get it into my debugger. So
> I'm sure this is the problem.
Excellent. So you saw is_open() be false, even though --log-bin was
given. That means RESET MASTER makes is_open() be false for an
instant. And so during this instant some statement can miss going to
the binlog (as it does a first check without mutex and then a second
check with mutex, like in mysql_insert()). Which is not an issue as
RESET MASTER deletes binlogs anyway...
Still, wasn't there value in testing is_open() in assertions? It was
to protect against wrong usage of binlog functions.
If we wanted to keep the testing of is_open() in assertions, we should
do it under mutex. How about introducing
is_open_for_debug() which would be like is_open() but uses the mutex?
And then replace, in DBUG_ASSERT(), is_open() by is_open_for_debug()?
Then:
- we have no crashes (bug is fixed) in debug builds as we use the
mutex
- we haven't slowed down release builds
... ?
