Davi,
> Andrei Elkin wrote:
>
> [..]
>
>>>>>>
>>>>>> 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.
>
> IMHO, this is wrong. If every process were to leave it's temporary file
> cleanup to the caller /tmp would be messier than it already is.
> Temporary files should be cleaned up unless the program crashed.
>
I guess this discussion went to the dead end:
I am saying there is no temprory files, you keep to stress on that.
>>>>>> 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.
>
> Not robust, on the contrary.
>
>> In cases you are pointing out the file perhaps could be safely
>> removed. You are welcome to make it better!
>
> Also, you are welcome to properly fix it :)
To fix what?
There is a possibility to create one less file if mysqlbinlog program
fails in Load_log_processor::process_first_event(). This part
does not belong to the bug issue bug#35546.
The first complaint
- mysqlbinlog.cc leaves temporary files on the
disk after shutdown. That leads to the pollution of temporary directory.
of bug#35543 is not precisely written.
It's not mysqlbinlog.cc but mtr that leaves the files.
There is no such thing as mysqlbinlog temporary file :)
And after the patch mtr removes the files.
If there is something else that you need, then you are welcome to file a
bug. It's not a secret there are lots of minor and not so minor things
to improve in mysqlbinlog. But distantly possible one-less file in caller's
temporary directory, that is supposed to be removed anyway, would be
certainly not the first item on my sorted list.
regards,
Andrei
>
>> 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?
>
> Everything with "temporary file" in that file. You say that there is no
> such thing as "mysqlbinlog temprory file", yet this expression is used a
> lot in the very same file.
>
>> create_unique_file?
>>
>> Does not the name says it precisely, that it's not about to create a
>> temporary file?
>
> /*
> ..
>
> Creates a temporary file to be used in LOAD DATA and writes first
> block of data to it. Registers its file name (and optional
> Create_file event) in the array of active temporary files.
>
> ..
>
> */
>
> --
>
> I think I've stated my point, now it's up to you and the reviewers. eof.
>
> Regards,
>
> --
> Davi Arnaut, Software Engineer
> MySQL Server Runtime Team
> Database Group, Sun Microsystems
