Sergey,
> Hi Andrei,
>
> The patch is ok to push. Below is another idea how to make it even better, but
> it's up to you to decided whether or not to implement it.
>
> On Mon, Mar 17, 2008 at 05:56:58PM +0200, Andrei Elkin wrote:
>> @@ -1209,32 +1211,82 @@ int MYSQL_LOG::purge_logs(const char *to
>> !log_in_use(log_info.log_file_name))
>> {
>> ulong file_size= 0;
>> - if (decrease_log_space) //stat the file we want to delete
>> + MY_STAT s, *lstat= my_stat(log_info.log_file_name, &s, MYF(0));
>> + if (!lstat)
>> {
>> - MY_STAT s;
>> -
>> - /*
>> - If we could not stat, we can't know the amount
>> - of space that deletion will free. In most cases,
>> - deletion won't work either, so it's not a problem.
>> - */
>> - if (my_stat(log_info.log_file_name,&s,MYF(0)))
>> - file_size= s.st_size;
>> - else
>> - sql_print_information("Failed to execute my_stat on file '%s'",
>> + if (my_errno == ENOENT)
>> + {
>> + /*
>> + It's not fatal if we can't stat a log file that does not exist;
>> + If we could not stat, we can't know the amount
>> + of space that deletion will free. In most cases,
>> + deletion won't work either, so it's not a problem.
> As we do not attempt to remove a file if my_stat() fails, this comment
> sounds a little bit misleading. Suggest to remove at least last sentence.
>
nothing to disagree; the clause was an old comments copy/paste. fixed
>> + */
>> + push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
>> + ER_LOG_PURGE_NO_FILE, ER(ER_LOG_PURGE_NO_FILE),
>> + log_info.log_file_name);
>> + sql_print_information("Failed to execute my_stat on file '%s'",
>> log_info.log_file_name);
>> + my_errno= 0;
>> + }
>> + else
>> + {
>> + /*
>> + Other than ENOENT are fatal
>> + */
>> + push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_ERROR,
>> + ER_BINLOG_PURGE_FATAL_ERR,
>> + "a problem with getting info on being purged %s; "
>> + "consider examining correspondence "
>> + "of your binlog index file "
>> + "to the actual binlog files",
>> + log_info.log_file_name);
>> + error= LOG_INFO_FATAL;
>> + goto err;
>> + }
>> + }
> -----------------------
>> + else if (decrease_log_space)
>> + {
>> + file_size= s.st_size;
>> }
>> - /*
>> - It's not fatal if we can't delete a log file ;
>> - if we could delete it, take its size into account
>> - */
>> +
>> DBUG_PRINT("info",("purging %s",log_info.log_file_name));
>> - if (!my_delete(log_info.log_file_name, MYF(0)) &&
> decrease_log_space)
>> - *decrease_log_space-= file_size;
>> + if (lstat)
>> + {
> -----------------------
> Now it is easy to combine these two conditions into one, like it is done in
> MYSQL_LOG::purge_logs_before_date(). That means:
actually that was me who have made it :) No reason to refuse this clean-up. Done
> - we'll save a few ticks by removing this if;
> - there is no need for variables like file_size and lstat anymore.
>
new patch has been committed.
cheers,
Andrei
