From: Date: May 2 2008 1:35pm
Subject: Re: bk commit into 6.0 tree (aelkin:1.2646) BUG#35546
List-Archive: http://lists.mysql.com/commits/46290
Message-Id: <87skx12b2q.fsf@koti.dsl.inet.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Davi, hello.

> Andrei Elkin wrote:
>> Below is the list of changes that have just been committed into a local
>> 6.0 repository of aelkin.  When aelkin does a push these changes
>> will be propagated to the main repository and, within 24 hours after the
>> push, to the public repository.
>> For information on how to access the public repository
>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>> 
>> ChangeSet@stripped, 2008-05-01 14:04:33+03:00, aelkin@stripped +2 -0
>>   Bug #35546 mysqlbinlog.cc: Load_log_processor::process_first_event() fails creating unique
>>   Bug #34283 mysqlbinlog leaves tmpfile after termination if binlog contains load data infile
>>   Bug #35543 mysqlbinlog.cc does not properly work with tmp files
>>   
>>   There were failures on pb executions in that mysqlbinlog could not compose a new 
>>   unique file name of its purpose.
>>   The reason of the bug has been that mysqlbinlog used the system wide temprorary directory instead
>>   of the specified by mtr so that mysqlbinlog's temporary files happened to be out of reach of mtr.
>>   
>>   Logics of the mysqlbinlog program correctly allows leaving new produced files such as ones that 
>>   hold LOAD DATA data.

>
> This is just working around the issue. What happens in a non-mtr
> environment? That tmp file creation function is unacceptable.
>

Let's state the issue precisely. Imo the issue is about removing
temprory files of mtr, not mysqlbinlog - bug#35546. mysqlbinlog is a
producer of some file(s) that are the main sql program and possibly
data files that the program can refer to e.g because of LOAD DATA. So
there are no temporary files as such that mysqlbinlog forgets to
remove. Removal is a duty of a caller of mysqlbinlog.

Let's agree on that first.

>>   This is fine as long as a caller of mysqlbinlog takes care of the new files later.
>
> OK, callers are responsible for cleaning up files left by mysqlbinlog.
>

And you seem to have agreed.

> How does a caller know which files mysqlbinlog created?

<paster from the patch>

  mysql-test/mysql-test-run.pl@stripped, 2008-05-01 14:04:31+03:00,aelkin@stripped +33 -1
    adding --local-load= $opt_tmpdir argument to mysqlbinlog.
</paste>

It can find out what has left in the mysqlbinlog's --local-load directory.

>
> How the caller know which files are temporary?

I understand you must have been confused with the synopsis
 
  Bug #34283 mysqlbinlog leaves tmpfile after termination

There is no such thing as mysqlbinlog temprory file. All the produced
files life time is the same as the main sql file.
The mysqlbinlog caller has not to forget to remove the main as well as
the rest.

>
> How the caller know which files were created by the mysqlbinlog instance
> he called?

The caller launches mysqlbinlog(s) with --local-load = some_location.
It should be possible and  easy for the caller to find a way how to
distinguish: e.g --local-load = `mkdir some new temp directory`

>
> IMHO, this is wrong. Temporary files should be erased by the one who
> created it and not the caller.
>

There's be nothing to disagree, but again there is no mysqlbinlog
temporary files really.

>>   The problem fixed with specifying explicitly the temp directory for mysqlbinlog when it's called
>>   from mtr. New files mysqlbinlog can produce now reside in the mtr temporary directory.
>
> This is not a proper fix.

I hope you change you mind after my clarifications.

>
> You also haven't fixed the other remarks on Bug#35543.
>

Thanks for noting!

This

  - There is also a kind of memory leak on the error path from
    create_unique_file().

has been dropped from my attention.
I am looking at this sub-issue now.

cheers,

Andrei