MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:October 15 2010 8:15am
Subject:Re: bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288
View as plain text  
Hi Luis,

It is up to you.
Just push the patch to 5.1.

Cheers.

On 10/15/2010 07:45 AM, Luís Soares wrote:
> Hi Alfranio,
>
> On 10/14/2010 05:16 PM, Alfranio Correia wrote:
>> Hi Luis,
>>
>> Great work. Patch approved.
>
> Thank you.
>
>> Please, report a bug to 5.1 and backport the patch.
>
> I can report a bug, or I can ask re-triage for this one, so that
> it gets into 5.1. Why not use this bug report ?  Perhaps you're
> thinking that the test failure is mostly due to a regression
> introduced by WL#2687 - which exists only in 5.5+ - we should use
> this one to fix 5.5+ and a new one to fix 5.1 ?
>>
>> The problem described in this bug report does not happen in 5.1
>> but your patch fixes a more severe problem.
>
> Yes, it does not happen in 5.1. Yes, it is severe, although with
> little impact I would say.
>
> Note that this patch fixes the case for a race between RESET
> MASTER and other threads that are writing to the binary log. I
> don't expect RESET MASTER to be called every now and then, should
> be a rare event.  Rotating the binary log, however should be more
> frequent, but, checking the code, it sets the correct flags (see
> new_file_impl):
>
>    - close(LOG_CLOSE_TO_BE_OPENED | LOG_CLOSE_INDEX);"
>
> So... no problem here.
>
> All in all, I would say that the impact in 5.1 is rather limited
> when compared to 5.5 (mostly because: 1. is_open is protected by
> the lock_log in MYSQL_BIN_LOG::write in 5.1, whereas in 5.5 is
> not).
>
>> As I understand is_open() may return false and your patch fixes
>> this as follows:
>>
>> -  close(LOG_CLOSE_INDEX);
>> +  close(LOG_CLOSE_INDEX | LOG_CLOSE_TO_BE_OPENED);
>
> Yes.
>
>> So if is_open() may return false, there may be changes that do not
>> get into the binary log because one expects that is_open never returns
>> false if the binary log is enabled. In other words, there are several
>> calls spread over the source code that uses the is_open() to decide
>> if changes should be written to the binary log. However, such calls
>> to is_open() are not protected through a mutex.
>
> OK.
>
>> Finally, the problem described in this bug report does not happen
>> in 5.1 because in ::write(...), there is a mutex protecting the call
>> to is_open().
>
> Yes, that's correct.
>
> Regards,
> Luís Soares
>
>> Cheers.
>>
>> On 10/13/2010 03:17 PM, Luis Soares wrote:
>>> #At
>>> file:///home/lsoares/Workspace/bzr/work/bugfixing/57288/mysql-5.5-bugteam/
>>> based on
>>> revid:alexander.nozdrin@stripped
>>>
>>>    3241 Luis Soares    2010-10-13 [merge]
>>>         BUG#57288: binlog_tmp_table fails sporadically: "Failed to write
>>>         the DROP statement ..."
>>>
>>>         Problem: When using temporary tables and closing a session, an
>>>         implicit DROP TEMPORARY TABLE IF EXISTS is written to the binary
>>>         log (while cleaning up the context of the session THD - see:
>>>         sql_class.cc:THD::cleanup which calls close_temporary_tables).
>>>
>>>         close_temporary_tables, first checks if the binary log is opened
>>>         and then proceeds to creating the DROP statements. Then, such
>>>         statements, are written to the binary log through
>>>         MYSQL_BIN_LOG::write(Log_event *). Inside, there is another check
>>>         if the binary log is opened and if not an error is returned. This
>>>         is where the faulty behavior is triggered. Given that the test
>>>         case replays a binary log, with temp tables statements, and right
>>>         after it issues RESET MASTER, there is a chance that is_open will
>>>         report false (when the mysql session is closed and the temporary
>>>         tables are written).
>>>
>>>         is_open may return false, because MYSQL_BIN_LOG::reset_logs is
>>>         not setting the correct flag (LOG_CLOSE_TO_BE_OPENED), on the
>>>         MYSQL_LOG_BIN::log_state (instead it sets just the
>>>         LOG_CLOSE_INDEX flag, leaving the log_state to
>>>         LOG_CLOSED). Thence, when writing the DROP statement as part of
>>>         the THD::cleanup, the thread could get a return value of false
>>>         for is_open - inside MYSQL_BIN_LOG::write, ultimately reporting
>>>         that it can't write the event to the binary log.
>>>
>>>         Fix: We fix this by adding the correct flag, missing in the
>>>         second close. Additionally, we also make the assignment of
>>>         log_file to be protected inside MYSQL_BIN_LOG::write, which could
>>>         also lead to potential races.
>>>
>>>       modified:
>>>         sql/log.cc
>>> === modified file 'sql/log.cc'
>>> --- a/sql/log.cc    2010-09-29 14:26:32 +0000
>>> +++ b/sql/log.cc    2010-10-13 14:15:36 +0000
>>> @@ -3295,7 +3295,7 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd)
>>>      }
>>>
>>>      /* Start logging with a new file */
>>> -  close(LOG_CLOSE_INDEX);
>>> +  close(LOG_CLOSE_INDEX | LOG_CLOSE_TO_BE_OPENED);
>>>      if ((error= my_delete_allow_opened(index_file_name, MYF(0))))
>>> // Reset (open will update)
>>>      {
>>>        if (my_errno == ENOENT)
>>> @@ -4693,8 +4693,8 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>>>
>>>        if (event_info->use_direct_logging())
>>>        {
>>> -      file=&log_file;
>>>          mysql_mutex_lock(&LOCK_log);
>>> +      file=&log_file;
>>>        }
>>>        else
>>>        {
>>>
>>>
>>>
>>>
>>>
>>
>

Thread
bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288Luis Soares13 Oct
  • Re: bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288Alfranio Correia14 Oct
    • Re: bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288Luís Soares15 Oct
      • Re: bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288Alfranio Correia15 Oct
  • Re: bzr commit into mysql-5.5-bugteam branch (luis.soares:3241) Bug#57288Sven Sandberg20 Oct