List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:March 22 2011 3:11pm
Subject:Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844
View as plain text  
Hi Magnus,

Thanks for fixing this. The patch is mostly ok, but would you mind doing 
the following:

  - I think binlog_func_foreach should return int, because both the 
functions that call it and the functions it calls return int, and the 
value is propagated to callers.

  - Since you do this change now, would you mind doing the same to 
ha_binlog_end?

/Sven


On 02/01/2011 10:48 AM, Magnus Blåudd wrote:
> #At file:///home/msvensson/mysql/trunk-bug59844/ based on
> revid:jon.hauglid@stripped
>
>   3580 Magnus Blåudd	2011-02-01
>        Bug#59844	Return value of ndb binlog functions are not checked
>         - return error code from ha_reset_slave, ha_reset_logs, ha_binlog_end and
>          ha_binlog_index_purge_file functions.
>         - check and handler return code from ha_reset_logs and ha_reset_slave
>         - Since ha_binlog_end is called during shutdown there is no error handling
>          to perform, the function should have been void.
>         - The ha_binlog_index_purge_file function might be necessary to handle
>           the return code from.
>
>      modified:
>        sql/binlog.cc
>        sql/handler.cc
>        sql/handler.h
>        sql/rpl_slave.cc
> === modified file 'sql/binlog.cc'
> --- a/sql/binlog.cc	2010-12-17 02:01:32 +0000
> +++ b/sql/binlog.cc	2011-02-01 09:48:52 +0000
> @@ -2130,7 +2130,18 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd)
>     const char* save_name;
>     DBUG_ENTER("reset_logs");
>
> -  ha_reset_logs(thd);
> +  if (ha_reset_logs(thd))
> +  {
> +    /*
> +      Failed to reset logs in handler(which should have pushed warnings
> +      describing the problem in more detail) ->  raise fatal binlog
> +      purge error
> +    */
> +    thd->raise_error(ER_BINLOG_PURGE_FATAL_ERR);
> +
> +    DBUG_RETURN(1);
> +  }
> +
>     /*
>       We need to get both locks to be sure that no one is trying to
>       write to the index log file.
>
> === modified file 'sql/handler.cc'
> --- a/sql/handler.cc	2011-01-21 10:50:31 +0000
> +++ b/sql/handler.cc	2011-02-01 09:48:52 +0000
> @@ -4231,27 +4231,28 @@ static my_bool binlog_func_foreach(THD *
>   {
>     hton_list_st hton_list;
>     uint i, sz;
> +  int res= 0;
>
>     hton_list.sz= 0;
>     plugin_foreach(thd, binlog_func_list,
>                    MYSQL_STORAGE_ENGINE_PLUGIN,&hton_list);
>
>     for (i= 0, sz= hton_list.sz; i<  sz ; i++)
> -    hton_list.hton[i]->binlog_func(hton_list.hton[i], thd, bfn->fn,
> bfn->arg);
> -  return FALSE;
> +    res |= hton_list.hton[i]->binlog_func(hton_list.hton[i], thd,
> +                                          bfn->fn, bfn->arg);
> +  return res != 0;
>   }
>
>   int ha_reset_logs(THD *thd)
>   {
>     binlog_func_st bfn= {BFN_RESET_LOGS, 0};
> -  binlog_func_foreach(thd,&bfn);
> -  return 0;
> +  return binlog_func_foreach(thd,&bfn);
>   }
>
> -void ha_reset_slave(THD* thd)
> +int ha_reset_slave(THD* thd)
>   {
>     binlog_func_st bfn= {BFN_RESET_SLAVE, 0};
> -  binlog_func_foreach(thd,&bfn);
> +  return binlog_func_foreach(thd,&bfn);
>   }
>
>   void ha_binlog_wait(THD* thd)
> @@ -4263,15 +4264,13 @@ void ha_binlog_wait(THD* thd)
>   int ha_binlog_end(THD* thd)
>   {
>     binlog_func_st bfn= {BFN_BINLOG_END, 0};
> -  binlog_func_foreach(thd,&bfn);
> -  return 0;
> +  return binlog_func_foreach(thd,&bfn);
>   }
>
>   int ha_binlog_index_purge_file(THD *thd, const char *file)
>   {
>     binlog_func_st bfn= {BFN_BINLOG_PURGE_FILE, (void *)file};
> -  binlog_func_foreach(thd,&bfn);
> -  return 0;
> +  return binlog_func_foreach(thd,&bfn);
>   }
>
>   struct binlog_log_query_st
>
> === modified file 'sql/handler.h'
> --- a/sql/handler.h	2011-01-26 21:12:56 +0000
> +++ b/sql/handler.h	2011-02-01 09:48:52 +0000
> @@ -2607,7 +2607,7 @@ void trans_register_ha(THD *thd, bool al
>   #ifdef HAVE_NDB_BINLOG
>   int ha_reset_logs(THD *thd);
>   int ha_binlog_index_purge_file(THD *thd, const char *file);
> -void ha_reset_slave(THD *thd);
> +int ha_reset_slave(THD *thd);
>   void ha_binlog_log_query(THD *thd, handlerton *db_type,
>                            enum_binlog_command binlog_command,
>                            const char *query, uint query_length,
> @@ -2615,12 +2615,13 @@ void ha_binlog_log_query(THD *thd, handl
>   void ha_binlog_wait(THD *thd);
>   int ha_binlog_end(THD *thd);
>   #else
> -#define ha_reset_logs(a) do {} while (0)
> -#define ha_binlog_index_purge_file(a,b) do {} while (0)
> -#define ha_reset_slave(a) do {} while (0)
> +static inline int ha_int_dummy(void) { return 0; }
> +#define ha_reset_logs(a) ha_int_dummy()
> +#define ha_binlog_index_purge_file(a,b) ha_int_dummy()
> +#define ha_reset_slave(a) ha_int_dummy()
>   #define ha_binlog_log_query(a,b,c,d,e,f,g) do {} while (0)
>   #define ha_binlog_wait(a) do {} while (0)
> -#define ha_binlog_end(a)  do {} while (0)
> +#define ha_binlog_end(a) ha_int_dummy()
>   #endif
>
>   const char *get_canonical_filename(handler *file, const char *path,
>
> === modified file 'sql/rpl_slave.cc'
> --- a/sql/rpl_slave.cc	2011-01-24 03:58:22 +0000
> +++ b/sql/rpl_slave.cc	2011-02-01 09:48:52 +0000
> @@ -5789,7 +5789,16 @@ int reset_slave(THD *thd, Master_info* m
>       goto err;
>     }
>
> -  ha_reset_slave(thd);
> +  if ((error= ha_reset_slave(thd)))
> +  {
> +    /*
> +      Failed to reset slave in handler(which should have pushed warnings
> +      describing the problem in more detail) ->  use the unknown error
> +      code already set
> +    */
> +    errmsg= "ha_reset_slave failed";
> +    goto err;
> +  }
>
>     // delete relay logs, clear relay log coordinates
>     if ((error= mi->rli->purge_relay_logs(thd,
>
>
>
>
>

Thread
bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Magnus Blåudd1 Feb
  • Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Sven Sandberg22 Mar
    • Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Magnus Blåudd23 Mar
Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Luís Soares8 Feb
  • Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Magnus Blåudd15 Feb
    • Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Luís Soares16 Jun
      • Re: bzr commit into mysql-trunk branch (magnus.blaudd:3580) Bug#59844Magnus Blåudd16 Jun