MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:March 12 2008 7:19pm
Subject:RE: bk commit into 6.0 tree (rafal:1.2580) WL#4212
View as plain text  
Rafal,

One question about this patch. 

> -----Original Message-----
> From: rsomla@stripped [mailto:rsomla@stripped] 
> Sent: Tuesday, March 11, 2008 6:38 AM
> To: commits@stripped
> Subject: bk commit into 6.0 tree (rafal:1.2580) WL#4212
> 
> Below is the list of changes that have just been committed 
> into a local 6.0 repository of rafal.  When rafal does a push 
> these changes will be propagated to the main repository and, 
> within 24 hours after the push, to the public repository.
> For information on how to access the public repository see 
> http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> 
> ChangeSet@stripped, 2008-03-11 11:37:40+01:00, rafal@quant.(none) +1 -0
>   WL#4212 (Online Backup : Kernel updates for object metadata changes)
>   
>   This patch fixes a problem detected by valgrind. When 
> binlog position was stored inside Image_info object, only
>   poitner to binlog file path stored in LOG_INFO was saved. 
> This was not correct, since the path stored inside
>   LOG_INFO can change. This is fixed by storing copy of the 
> file path in a String buffer inside Image_info object.
> 
>   sql/backup/image_info.h@stripped, 2008-03-11 11:37:36+01:00, 
> rafal@quant.(none) +7 -1
>     Added m_binlog_file member to Image_info class.
>     When saving binlog position, save binlog file name in the 
> m_binlog_file member (as it can change later).
> 
> diff -Nrup a/sql/backup/image_info.h b/sql/backup/image_info.h
> --- a/sql/backup/image_info.h	2008-03-11 10:00:16 +01:00
> +++ b/sql/backup/image_info.h	2008-03-11 11:37:36 +01:00
> @@ -132,6 +132,7 @@ public: // public interface
>  
>    MEM_ROOT  mem_root;    ///< Memory root used to allocate 
> @c Obj instances.
>    Map<uint, Db>   m_dbs; ///< Pointers to Db instances.
> +  String    m_binlog_file; ///< To store binlog file name at VP time.
>  
>    // friends
>  
> @@ -701,8 +702,13 @@ void Image_info::save_vp_time(const time 
>  inline  void Image_info::save_binlog_pos(const ::LOG_INFO &li)  {
> +  // save current binlog file name
> +  m_binlog_file.length(0);
> +  m_binlog_file.append(li.log_file_name);
> +
> +  // store binlog coordinates
>    binlog_pos.pos= (unsigned long int)li.pos;
> -  binlog_pos.file= const_cast<char*>(li.log_file_name);
> +  binlog_pos.file= const_cast<char*>(m_binlog_file.ptr());
>  }
>  

I see you are storing a copy of the file name instead of referring to it. Do
you expect or can you think of scenarios where the binlog filename can
change during backup? If so, I think they would be better handled in
WL#4209.

Chuck

Thread
bk commit into 6.0 tree (rafal:1.2580) WL#4212rsomla11 Mar
  • RE: bk commit into 6.0 tree (rafal:1.2580) WL#4212Chuck Bell12 Mar