STATUS
======
Patch approve.
COMMENTARY
==========
Generally, I think that errors should be reported at the lowest possible
level where you most often know more about the details of the problem.
Hence, I would have preferred that errors where reported from within
block_commits. However, since there does not seem to be much more
information available there about potential errors, I don't think that
is very important here.
--
Øystein
Jorgen Loland wrote:
> #At file:///localhome/jl208045/mysql/mysql-6.0-backup-40970/
>
> 2737 Jorgen Loland 2008-12-01
> Bug#40970 - Errors from commit blocker are not logged.
>
> If block_commits fails, an error is now logged. unblock_commits cannot fail.
> modified:
> sql/backup/data_backup.cc
> sql/share/errmsg.txt
>
> per-file messages:
> sql/backup/data_backup.cc
> Log error if block_commits fails.
> sql/share/errmsg.txt
> Added error message for when backup fails to block commits
> === modified file 'sql/backup/data_backup.cc'
> --- a/sql/backup/data_backup.cc 2008-11-26 10:05:19 +0000
> +++ b/sql/backup/data_backup.cc 2008-12-01 10:12:45 +0000
> @@ -402,13 +402,13 @@ int block_commits(THD *thd, TABLE_LIST *
>
> @param thd (in) the current thread structure.
>
> - @returns 0
> + This method cannot fail.
> */
> -int unblock_commits(THD *thd)
> +void unblock_commits(THD *thd)
> {
> DBUG_ENTER("unblock_commits()");
> unlock_global_read_lock(thd);
> - DBUG_RETURN(0);
> + DBUG_VOID_RETURN;
> }
>
> /**
> @@ -652,7 +652,10 @@ int write_table_data(THD* thd, Backup_in
> int error= 0;
> error= block_commits(thd, NULL);
> if (error)
> + {
> + log.report_error(ER_BACKUP_BLOCK_COMMITS);
> goto error;
> + }
>
> if (sch.prepare()) // logs errors
> goto error;
> @@ -691,9 +694,7 @@ int write_table_data(THD* thd, Backup_in
> Unblock commits.
> */
> DEBUG_SYNC(thd, "before_backup_unblock_commit");
> - error= unblock_commits(thd);
> - if (error)
> - goto error;
> + unblock_commits(thd);
>
> report_vp_info(info);
>
>
> === modified file 'sql/share/errmsg.txt'
> --- a/sql/share/errmsg.txt 2008-11-28 10:10:39 +0000
> +++ b/sql/share/errmsg.txt 2008-12-01 10:12:45 +0000
> @@ -6442,3 +6442,6 @@ ER_BACKUP_BACKUP_DBS
> eng "Backing up %u database(s) %.220s"
> ER_BACKUP_RESTORE_DBS
> eng "Restoring %u database(s) %.220s"
> +ER_BACKUP_BLOCK_COMMITS
> + eng "Backup failed to set commit blocker."
> +
>
>
--
Øystein Grøvlen, Senior Staff Engineer
Sun Microsystems, Database Group
Trondheim, Norway
