Re: bk commit into 6.0 tree (aelkin:1.2646) BUG#35546

[Andrei Elkin]

Davi,

>>>> 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.

And I myself got trapped to write a misleading clause.

It should be

       of the specified by mtr so that files that mysqlbinlog produced 
       happened to be out of reach of mtr.

instead.

>>>>   
>>>>   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.
>
> I partly agree. I agree that the caller should clean it if the operation
> finished successfully, otherwise it's mysqlbinlog duty to clean up.

If mysqlbinlog finished with a failure, there would be a subset of
files it was going to produce. 
Whether the subset is whole or not is
out of mysqlbinlog concern.  The user might be okay with partial
results and possibly would like to process them further.
The user/a binlog caller are gaining all the info with the current
patch in order to determine where are the files mysqlbinlog has
produced.
However it has been their concern so far to process them further and
possibly to remove.


>
>>>>   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
>

> I haven't looked at Bug#34283 but I might be misunderstanding something.
> They way I see it is that mysqlbinlog creates a few temporary files (by
> calling create_unique_file) but does not remove the temp file if the
> operation fails later on. For example, the function
> Load_log_processor::process_first_event:
>
>
>   if ((file= create_unique_file(fname,ptr)) < 0)
>   {
>     error("Could not construct local filename %s%s.",
>           target_dir_name,bname);
>     delete ce;
>     DBUG_RETURN(ERROR_STOP);
>   }
>
>   rec.fname= fname;
>   rec.event= ce;
>
>   if (set_dynamic(&file_names, (uchar*)&rec, file_id))
>   {
>     error("Out of memory.");
>     delete ce;
>     DBUG_RETURN(ERROR_STOP);
>   }
>
> The problem of this and other error paths is that the temporary file is
> left behind without proper cleanup.
>

I think the current logic is pretty robust although not ideal.
In cases you are pointing out the file perhaps could be safely
removed.
You are welcome to make it better!

But this is not something that affected cases of the bugs I am fixing.
To remind - the matter is that not-failed mysqlbinlog saturated a
directory that was not under the mtr's supervision.

>> There is no such thing as mysqlbinlog temprory file.
>
> If there is no temporary file the function name should be changed to
> something more meaningful.

Which function do you mean?

create_unique_file?

Does not the name says it precisely, that it's not about to create a
temporary file?

cheers,

Andrei