Re: bk commit into 6.0 tree (cbell:1.2600) BUG#34858

[Rafal Somla]

Chuck,

I don't see any major problems with this patch. See some minor comments and 
questions below.

cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 6.0 repository of cbell.  When cbell 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-19 12:56:57-04:00, cbell@mysql_cab_desk. +9 -0
>   BUG#34858 Server crashes by backup tests 
>   
>   Patch fixes problems encountered when merging runtime code changes
>   into main for online backup.
>   
>   * snapshot driver fixed
>   * progress table access changed to that of performance tables (logs)
> 
>   mysql-test/r/backup_progress.result@stripped, 2008-03-19 12:56:40-04:00,
> cbell@mysql_cab_desk. +6 -3
>     BUG#34858 Server crashes by backup tests 
>     
>     New result file.
> 
>   mysql-test/t/backup_progress.test@stripped, 2008-03-19 12:56:41-04:00,
> cbell@mysql_cab_desk. +8 -3
>     BUG#34858 Server crashes by backup tests 
>     
>     Test updates due to different structure of tables (they're
>     logs now and cannot be locked).
> 
>   mysql-test/t/disabled.def@stripped, 2008-03-19 12:56:41-04:00, cbell@mysql_cab_desk.
> +0 -5
>     BUG#34858 Server crashes by backup tests 
>         
>     Reenabled disabled tests.
> 
>   sql/backup/backup_progress.cc@stripped, 2008-03-19 12:56:43-04:00,
> cbell@mysql_cab_desk. +498 -597
>     BUG#34858 Server crashes by backup tests 
>      
>     Refactored code to use performance table access methods for the
>     backup logs.
> 
>   sql/backup/backup_progress.h@stripped, 2008-03-19 12:56:44-04:00, cbell@mysql_cab_desk.
> +88 -60
>     BUG#34858 Server crashes by backup tests 
>     
>     Added new methods for writing to backup logs.
>     Changed enums to prevent collisions with server code.
> 
>   sql/backup/be_snapshot.cc@stripped, 2008-03-19 12:56:44-04:00, cbell@mysql_cab_desk. +1
> -0
>     BUG#34858 Server crashes by backup tests 
>     
>     Added code to perform commit of consistent read to avoid another
>     problem introduced from changes to handler.cc.
> 
>   sql/backup/data_backup.cc@stripped, 2008-03-19 12:56:45-04:00, cbell@mysql_cab_desk. +7
> -7
>     BUG#34858 Server crashes by backup tests 
>     
>     Added THD reference to all report_ob* calls.
>     
> 
>   sql/backup/kernel.cc@stripped, 2008-03-19 12:56:46-04:00, cbell@mysql_cab_desk. +24
> -22
>     BUG#34858 Server crashes by backup tests 
>     
>     Added THD reference to all report_ob* calls.
>     
> 
>   sql/table.cc@stripped, 2008-03-19 12:56:42-04:00, cbell@mysql_cab_desk. +19 -0
>     BUG#34858 Server crashes by backup tests 
>     
>     Added code to process the backup logs tables as performance tables
>     using the original names from the original backup progress code.
> 
==================================================================================
> diff -Nrup a/mysql-test/r/backup_progress.result
> b/mysql-test/r/backup_progress.result
> --- a/mysql-test/r/backup_progress.result	2008-02-14 13:14:49 -05:00
> +++ b/mysql-test/r/backup_progress.result	2008-03-19 12:56:40 -04:00
> @@ -32,7 +32,8 @@ con2: Send backup command.
>  BACKUP DATABASE backup_progress to 'backup_progress_orig.bak';
>  con1: Checking locks.
>  con1: Checking progress.
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "BACKUP DATABASE backup_progress%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "BACKUP DATABASE backup_progress%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  SELECT backup_state FROM mysql.online_backup AS ob JOIN backup_progress.t1_res as t1
> ON ob.backup_id = t1.id;
>  backup_state
>  starting
> @@ -107,7 +108,8 @@ con1: Checking locks.
>  con1: Checking progress.
>  select * from backup_progress.t1_res;
>  id
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "RESTORE FROM 'backup_progress_orig.bak'%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "RESTORE FROM%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  SELECT backup_state FROM mysql.online_backup AS ob JOIN backup_progress.t1_res as t1
> ON ob.backup_id = t1.id;
>  backup_state
>  starting
> @@ -128,7 +130,8 @@ con2: Finish restore command
>  backup_id
>  #
>  DELETE FROM backup_progress.t1_res;
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "RESTORE FROM 'backup_progress_orig.bak'%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "RESTORE FROM%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  SELECT ob.* FROM mysql.online_backup AS ob JOIN backup_progress.t1_res AS t1 ON
> ob.backup_id = t1.id;;
>  backup_id	#
>  process_id	#
==================================================================================
> diff -Nrup a/mysql-test/t/backup_progress.test b/mysql-test/t/backup_progress.test
> --- a/mysql-test/t/backup_progress.test	2008-02-14 13:16:02 -05:00
> +++ b/mysql-test/t/backup_progress.test	2008-03-19 12:56:41 -04:00
> @@ -66,6 +66,7 @@ connection con2;
>  send BACKUP DATABASE backup_progress to 'backup_progress_orig.bak';
>  
>  connection con1;
> +
>  # Wait for lock to be acquired and execution to reach breakpoint
>  --echo con1: Checking locks.
>  let $wait_condition = SELECT state = "debug_sync_point: bp_starting_state"
> @@ -81,7 +82,8 @@ let $wait_condition = SELECT backup_stat
>  --source include/wait_condition.inc
>  
>  --echo: Display progress
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "BACKUP DATABASE backup_progress%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "BACKUP DATABASE backup_progress%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  SELECT backup_state FROM mysql.online_backup AS ob JOIN backup_progress.t1_res as t1
> ON ob.backup_id = t1.id;
>  
>  --echo con1: Advance the lock.
> @@ -172,6 +174,7 @@ DELETE FROM backup_progress.t1_res;
>  send RESTORE FROM 'backup_progress_orig.bak';
>  
>  connection con1;
> +
>  # Wait for lock to be acquired and execution to reach breakpoint
>  --echo con1: Checking locks.
>  let $wait_condition = SELECT state = "debug_sync_point: bp_starting_state"
> @@ -188,7 +191,8 @@ let $wait_condition = SELECT backup_stat
>  
>  --echo: Display progress
>  select * from backup_progress.t1_res;
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "RESTORE FROM 'backup_progress_orig.bak'%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "RESTORE FROM%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  SELECT backup_state FROM mysql.online_backup AS ob JOIN backup_progress.t1_res as t1
> ON ob.backup_id = t1.id;
>  
>  --echo con1: Advance the lock.
> @@ -221,7 +225,8 @@ reap;
>  
>  #Show results
>  DELETE FROM backup_progress.t1_res;
> -INSERT INTO backup_progress.t1_res (id) SELECT backup_id FROM mysql.online_backup
> WHERE command LIKE "RESTORE FROM 'backup_progress_orig.bak'%";
> +SELECT MAX(backup_id) INTO @bup_id FROM mysql.online_backup WHERE command LIKE
> "RESTORE FROM%";
> +INSERT INTO backup_progress.t1_res (id) VALUES (@bup_id);
>  --replace_column 1 # 2 # 3 # 4 # 10 # 11 # 12 #
>  --query_vertical SELECT ob.* FROM mysql.online_backup AS ob JOIN
> backup_progress.t1_res AS t1 ON ob.backup_id = t1.id;
>  --replace_column 1 # 3 # 4 #
==================================================================================
> diff -Nrup a/mysql-test/t/disabled.def b/mysql-test/t/disabled.def
> --- a/mysql-test/t/disabled.def	2008-03-18 10:18:53 -04:00
> +++ b/mysql-test/t/disabled.def	2008-03-19 12:56:41 -04:00
> @@ -33,8 +33,3 @@ backup_snapshot       :BUG#34235 pending
>  csv_alter_table      : Bug#33696 2008-01-21 pcrews no .result file - bug allows NULL
> columns in CSV tables
>  ctype_latin2_ch      : BUG #33791 2008-01-18 mats Wrong ORDER BY with
> latin2_czech_cs
>  
> -backup_errors        : Bug#34858 2008-Feb-28 Server crashes by backup tests
> -backup_fkey          : Bug#34858 2008-Feb-28 Server crashes by backup tests
> -backup_no_data       : Bug#34858 2008-Feb-28 Server crashes by backup tests
> -backup_no_engine     : Bug#34858 2008-Feb-28 Server crashes by backup tests
> -backup_objects       : Bug#34858 2008-Feb-28 Server crashes by backup tests
==================================================================================
> diff -Nrup a/sql/backup/backup_progress.cc b/sql/backup/backup_progress.cc
> --- a/sql/backup/backup_progress.cc	2008-02-26 15:18:12 -05:00
> +++ b/sql/backup/backup_progress.cc	2008-03-19 12:56:43 -04:00
> @@ -27,6 +27,12 @@
>  #include "backup_progress.h"
>  #include "be_thread.h"
>  
> +/* BACKUP_HISTORY_LOG name */
> +LEX_STRING BACKUP_HISTORY_LOG_NAME= {C_STRING_WITH_LEN("online_backup")};
> +
> +/* BACKUP_PROGRESS_LOG name */
> 
> +LEX_STRING BACKUP_PROGRESS_LOG_NAME= {C_STRING_WITH_LEN("online_backup_progress")};
> +
>  /**
>     Check online backup progress tables.
>  
> @@ -70,139 +76,6 @@ my_bool check_ob_progress_tables(THD *th
>  }
>  
>  /**
> -   Open backup progress table.
> -
> -   This method opens the online backup table specified. It uses the locking
> -   thread mechanism in be_thread.cc to open the table in a separate thread.
> -
> -   @param char *         table_name  The name of the table to open.
> -   @param thr_lock_type  lock        The lock type -- TL_WRITE or TL_READ.
> -
> -   @returns 0 = success
> -   @returns 1 = failed to open table
> -
> -   @todo : Replace poling loop with signal.
> -  */
> -Locking_thread_st *open_backup_progress_table(const char *table_name,
> -                                              enum thr_lock_type lock)
> -{
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  Locking_thread_st *locking_thd;       // The locking thread
> -
> -  DBUG_ENTER("open_backup_progress_table()");
> -
> -  /*
> -    The locking thread will, via open_table(), fail if the table does not
> -    exist.
> -  */
> -
> -  /*
> -    Create a new thread to open and lock the tables.
> -  */
> -  locking_thd= new Locking_thread_st();
> -  if (locking_thd == NULL)
> -    DBUG_RETURN(locking_thd);    
> -
> -  locking_thd->tables_in_backup= (TABLE_LIST*)my_malloc(sizeof(TABLE_LIST),
> MYF(MY_WME));
> -  locking_thd->tables_in_backup->init_one_table("mysql", table_name, lock);
> -
> -
> -  /*
> -    Start the locking thread and wait until it is ready.
> -  */
> -  locking_thd->start_locking_thread("backup progress locking thread");
> -
> -  /*
> -    Poll the locking thread until ready.
> -  */
> -  while (locking_thd && (locking_thd->lock_state != LOCK_ACQUIRED)
> &&
> -         (locking_thd->lock_state != LOCK_ERROR))
> -    my_sleep(1);
> -  if (locking_thd->lock_state == LOCK_ERROR)
> -  {
> -    delete locking_thd;
> -    locking_thd= NULL;
> -  }
> -  DBUG_RETURN(locking_thd);
> -}
> -
> -/**
> -   Close backup progress table.
> -
> -   This method closes the online backup table specified. It uses the locking
> -   thread mechanism in be_thread.cc to close the table in a separate thread.
> -
> -   @param Locking_thread_st *locking_thd  The locking thread.
> -
> -   @returns 0 = success
> -   @returns 1 = failed to close table
> -
> -   @todo : Replace poling loop with signal.
> -  */
> -int close_backup_progress_table(Locking_thread_st *locking_thd)
> -{
> -  DBUG_ENTER("close_backup_progress_table()");
> -
> -  /*
> -    Tell locking thread to die.
> -  */
> -  locking_thd->kill_locking_thread();
> -
> -  /*
> -    Poll the locking thread until ready.
> -  */
> -  while (locking_thd && (locking_thd->lock_state != LOCK_DONE)
> &&
> -         (locking_thd->lock_state != LOCK_ERROR))
> -    my_sleep(1);
> -  if (locking_thd->lock_state == LOCK_ERROR)
> -    DBUG_RETURN(1);
> -  my_free(locking_thd->tables_in_backup, MYF(0));
> -  delete locking_thd;
> -  locking_thd= NULL;
> -  DBUG_RETURN(0);
> -}
> -
> -/**
> -   Find the row in the table that matches the backup_id.
> -
> -   This method locates the row in the online backup table that matches the
> -   backup_id passed.
> -
> -   @param TABLE *    table      The table to search.
> -   @param ulonglong  backup_id  The id of the row to locate.
> -
> -   @returns 0 = success
> -   @returns 1 = failed to find row
> -  */
> -my_bool find_online_backup_row(TABLE *table, ulonglong backup_id)
> -{
> -  uchar key[MAX_KEY_LENGTH]; // key buffer for search
> -
> -  DBUG_ENTER("find_online_backup_row()");
> -
> -  /*
> -    Create key to find row. We have to use field->store() to be able to
> -    handle VARCHAR and CHAR fields.
> -    Assumption here is that the two first fields in the table are
> -    'db' and 'name' and the first key is the primary key over the
> -    same fields.
> -  */
> -  table->field[ET_FIELD_BACKUP_ID]->store(backup_id, TRUE);
> -
> -  key_copy(key, table->record[0], table->key_info,
> table->key_info->key_length);
> -
> -  if (table->file->index_read_idx_map(table->record[0], 0, key,
> HA_WHOLE_KEY,
> -                                      HA_READ_KEY_EXACT))
> -  {
> -    DBUG_PRINT("info", ("Row not found"));
> -    DBUG_RETURN(TRUE);
> -  }
> -
> -  DBUG_PRINT("info", ("Row found!"));
> -  DBUG_RETURN(FALSE);
> -}
> -
> -/**
>     Get text string for state.
>  
>     @param enum_backup_state  state        The current state of the operation
> @@ -241,260 +114,522 @@ void get_state_string(enum_backup_state 
>  }
>  
>  /**
> -   Update an integer field for the row in the table that matches the backup_id.
> +  Write the backup log entry for the backup history log to a table.
>  
> -   This method locates the row in the online backup table that matches the
> -   backup_id passed and updates the field specified.
> +  This method creates a new row in the backup history log with the
> +  information provided.
>  
> -   @param ulonglong            backup_id   The id of the row to locate.
> -   @param char *               table_name  The name of the table to open.
> -   @param enum_ob_table_field  fld         Field to update.
> -   @param ulonglong            value       Value to set field to.
> +  @param[IN]   thd          The current thread
> +  @param[OUT]  backup_id    The new row id for the backup history
> +  @param[IN]   process_id   The process id of the operation 
> +  @param[IN]   state        The current state of the operation
> +  @param[IN]   operation    The current state of the operation

Wrong description of "operation" parameter.

> +  @param[IN]   error_num    The error number
> +  @param[IN]   user_comment The user's comment specified in the
> +                            command (not implemented yet)
> +  @param[IN]   backup_file  The name of the target file
> +  @param[IN]   command      The actual command entered
> +
> +  @returns TRUE if error.
> +
> +  @todo Add internal error handler to handle errors that occur on
> +        open. See  thd->push_internal_handler(&error_handler).
> +*/
> +bool backup_history_log_write(THD *thd, 
> +                              ulonglong *backup_id,
> +                              int process_id,
> +                              enum_backup_state state,
> +                              enum_backup_operation operation,
> +                              int error_num,
> +                              const char *user_comment,
> +                              const char *backup_file,
> +                              const char *command)
> +{
> +  TABLE_LIST table_list;
> +  TABLE *table= NULL;
> +  bool result= TRUE;
> +  bool need_close= FALSE;
> +  bool need_pop= FALSE;
> +  bool need_rnd_end= FALSE;
> +  Open_tables_state open_tables_backup;
> +  bool save_time_zone_used;
> +  char *host= current_thd->security_ctx->host; // host name
> +  char *user= current_thd->security_ctx->user; // user name
>  
> -   @returns 0 = success
> -   @returns 1 = failed to find row
> +  save_time_zone_used= thd->time_zone_used;
> +  bzero(& table_list, sizeof(TABLE_LIST));
> +  table_list.alias= table_list.table_name= BACKUP_HISTORY_LOG_NAME.str;
> +  table_list.table_name_length= BACKUP_HISTORY_LOG_NAME.length;
> +
> +  table_list.lock_type= TL_WRITE_CONCURRENT_INSERT;
> +
> +  table_list.db= MYSQL_SCHEMA_NAME.str;
> +  table_list.db_length= MYSQL_SCHEMA_NAME.length;
> +
> +  if (!(table= open_performance_schema_table(thd, & table_list,
> +                                             & open_tables_backup)))
> +    goto err;
> +
> +  need_close= TRUE;
> +
> +  if (table->file->extra(HA_EXTRA_MARK_AS_LOG_TABLE) ||
> +      table->file->ha_rnd_init(0))
> +    goto err;
> +
> +  need_rnd_end= TRUE;
> +
> +  /* Honor next number columns if present */
> +  table->next_number_field= table->found_next_number_field;
> +
> +  /*
> +    Get defaults for new record.
>    */
> -int update_online_backup_int_field(ulonglong backup_id, 
> -                                   const char *table_name,
> -                                   enum_ob_table_field fld, 
> -                                   ulonglong value)
> -{
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  int ret= 0;                           // return value
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> +  restore_record(table, s->default_values); 
> +
> +  /* check that all columns exist */
> +  if (table->s->fields < 18)

Why not use the enum value from the "cool trick"?

> +    goto err;
> +
> +  /*
> +    Fill in the data.
> +  */
> +  table->field[ET_OBH_FIELD_PROCESS_ID]->store(process_id, TRUE);
> +  table->field[ET_OBH_FIELD_PROCESS_ID]->set_notnull();
> +  table->field[ET_OBH_FIELD_BACKUP_STATE]->store(state, TRUE);
> +  table->field[ET_OBH_FIELD_BACKUP_STATE]->set_notnull();
> +  table->field[ET_OBH_FIELD_OPER]->store(operation, TRUE);
> +  table->field[ET_OBH_FIELD_OPER]->set_notnull();
> +  table->field[ET_OBH_FIELD_ERROR_NUM]->store(error_num, TRUE);
> +  table->field[ET_OBH_FIELD_ERROR_NUM]->set_notnull();
>  
> -  DBUG_ENTER("update_int_field()");
> +  if (host)
> +  {
> +    if(table->field[ET_OBH_FIELD_HOST_OR_SERVER]->store(host, 
> +       strlen(host), system_charset_info))
> +      goto err;
> +    table->field[ET_OBH_FIELD_HOST_OR_SERVER]->set_notnull();
> +  }
>  
> -  locking_thd= open_backup_progress_table(table_name, TL_WRITE);
> -  if (!locking_thd)
> +  if (user)
>    {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> +    if (table->field[ET_OBH_FIELD_USERNAME]->store(user,
> +        strlen(user), system_charset_info))
> +      goto err;
> +    table->field[ET_OBH_FIELD_USERNAME]->set_notnull();
>    }
>  
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> +  if (user_comment)
> +  {
> +    if (table->field[ET_OBH_FIELD_COMMENT]->store(user_comment,
> +        strlen(user_comment), system_charset_info))
> +      goto err;
> +    table->field[ET_OBH_FIELD_COMMENT]->set_notnull();
> +  }
>  
> -  if (find_online_backup_row(table, backup_id))
> +  if (backup_file)
>    {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(1);
> +    if (table->field[ET_OBH_FIELD_BACKUP_FILE]->store(backup_file, 
> +        strlen(backup_file), system_charset_info))
> +      goto err;
> +    table->field[ET_OBH_FIELD_BACKUP_FILE]->set_notnull();
>    }
>  
> -  store_record(table, record[1]);
> +  if (command)
> +  {
> +    if (table->field[ET_OBH_FIELD_COMMAND]->store(command,
> +        strlen(command), system_charset_info))
> +      goto err;
> +    table->field[ET_OBH_FIELD_COMMAND]->set_notnull();
> +  }
>  
> -  /*
> -    Fill in the data.
> -  */
> -  table->field[fld]->store(value, TRUE);
> -  table->field[fld]->set_notnull();
> +  /* log table entries are not replicated */
> +  if (table->file->ha_write_row(table->record[0]))
> +    goto err;
>  
>    /*
> -    Update the row.
> +    Get last insert id for row.
>    */
> -  if ((ret= table->file->ha_update_row(table->record[1],
> table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> +  *backup_id= table->file->insert_id_for_cur_row;

Are you sure that table->file->insert_id_for_cur_row is the same as the value 
assigned to the auto-increment column. I vaguely recall that wierd things can 
happen to auto-increment values - for example one can change the initial value 
and increment step with server options. Would the above code be consistent also 
in that case?

>  
> -  ret= close_backup_progress_table(locking_thd);
> -  DBUG_RETURN(ret);
> +  result= FALSE;
> +
> +err:
> +  if (result && !thd->killed)
> +    sql_print_error("Failed to write to mysql.online_backup.");
> +
> +  if (need_rnd_end)
> +  {
> +    table->file->ha_rnd_end();
> +    table->file->ha_release_auto_increment();
> +  }
> +  if (need_close)
> +    close_performance_schema_table(thd, & open_tables_backup);
> +
> +  thd->time_zone_used= save_time_zone_used;
> +  return result;
>  }
>  
>  /**
> -   Update a datetime field for the row in the table that matches the backup_id.
> +  Update a backup history log entry for the given backup_id to a table.
>  
> -   This method locates the row in the online backup table that matches the
> -   backup_id passed and updates the field specified.
> +  This method updates a row in the backup history log using one
> +  of four data types as determined by the field (see fld).
>  
> -   @param ulonglong            backup_id   The id of the row to locate.
> -   @param char *               table_name  The name of the table to open.
> -   @param enum_ob_table_field  fld         Field to update.
> -   @param my_time_t            value       Value to set field to.
> +  @param[IN]   thd          The current thread
> +  @param[IN]   backup_id    The row id for the backup history to be updated
> +  @param[IN]   fld          The enum for the field to be updated 
> +  @param[IN]   val_long     The value for long fields
> +  @param[IN]   val_time     The value for time fields
> +  @param[IN]   val_str      The value for char * fields
> +  @param[IN]   val_state    The value for state fields
>  
> -   @returns 0 = success
> -   @returns 1 = failed to find row
> -  */
> -int update_online_backup_datetime_field(ulonglong backup_id, 
> -                                        const char *table_name,
> -                                        enum_ob_table_field fld, 
> -                                        time_t value)
> +  @returns TRUE if error.
> +
> +  @todo Add internal error handler to handle errors that occur on
> +        open. See  thd->push_internal_handler(&error_handler).
> +*/
> +bool backup_history_log_update(THD *thd, 
> +                               ulonglong backup_id,
> +                               enum_backup_history_table_field fld,
> +                               ulonglong val_long,
> +                               time_t val_time,
> +                               const char *val_str,
> +                               int val_state)
>  {
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  int ret= 0;                           // return value
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> +  TABLE_LIST table_list;
> +  TABLE *table= NULL;
> +  bool result= TRUE;
> +  bool need_close= FALSE;
> +  bool need_pop= FALSE;
> +  bool need_rnd_end= FALSE;
> +  Open_tables_state open_tables_backup;
> +  bool save_time_zone_used;
> +  int ret= 0;
>  
> -  DBUG_ENTER("update_int_field()");
> +  save_time_zone_used= thd->time_zone_used;
>  
> -  locking_thd= open_backup_progress_table(table_name, TL_WRITE);
> -  if (!locking_thd)
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> +  bzero(& table_list, sizeof(TABLE_LIST));
> +  table_list.alias= table_list.table_name= BACKUP_HISTORY_LOG_NAME.str;
> +  table_list.table_name_length= BACKUP_HISTORY_LOG_NAME.length;
>  
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> +  table_list.lock_type= TL_WRITE_CONCURRENT_INSERT;
>  
> -  if (find_online_backup_row(table, backup_id))
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(1);
> -  }
> +  table_list.db= MYSQL_SCHEMA_NAME.str;
> +  table_list.db_length= MYSQL_SCHEMA_NAME.length;
> +
> +  if (!(table= open_performance_schema_table(thd, & table_list,
> +                                             & open_tables_backup)))
> +    goto err;
> +

Above code shared with previous function. Perhaps good to refactor it into a 
helper function?

> +  if (find_backup_history_row(table, backup_id))
> +    goto err;
> +
> +  need_close= TRUE;
>  
>    store_record(table, record[1]);
>  
>    /*
>      Fill in the data.
>    */
> -  if (value)
> -  {
> -    MYSQL_TIME time;
> -    my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)value);
> -
> -    table->field[fld]->set_notnull();
> -    table->field[fld]->store_time(&time, MYSQL_TIMESTAMP_DATETIME);
> +  switch (fld) {
> +    case ET_OBH_FIELD_BINLOG_POS:
> +    case ET_OBH_FIELD_ERROR_NUM:
> +    case ET_OBH_FIELD_NUM_OBJ:
> +    case ET_OBH_FIELD_TOTAL_BYTES:
> +    {
> +      table->field[fld]->store(val_long, TRUE);
> +      table->field[fld]->set_notnull();
> +      break;
> +    }
> +    case ET_OBH_FIELD_BINLOG_FILE:
> +    {
> +      if (val_str)
> +      {
> +        if(table->field[fld]->store(val_str, strlen(val_str), 
> +                                    system_charset_info))
> +          goto err;
> +        table->field[fld]->set_notnull();
> +      }
> +      break;
> +    }    
> +    case ET_OBH_FIELD_ENGINES:
> +    {
> +      String str;    // engines string
> +      str.length(0);
> +      table->field[fld]->val_str(&str);
> +      if (str.length() > 0)
> +        str.append(", ");
> +      str.append(val_str);
> +      if (str.length() > 0)
> +      {
> +        if(table->field[fld]->store(str.c_ptr(), 
> +           str.length(), system_charset_info))
> +          goto err;
> +        table->field[fld]->set_notnull();
> +      }
> +      break;
> +    }    
> +    case ET_OBH_FIELD_START_TIME:
> +    case ET_OBH_FIELD_STOP_TIME:
> +    case ET_OBH_FIELD_VP:
> +    {
> +      if (val_time)
> +      {
> +        MYSQL_TIME time;
> +        my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)val_time);
> +

If we need value of type my_time_t, why the val_time parameter is not of that 
type? In general, perhaps we should use consistently my_time_t type, not time_t.

I see that this is the same code as was there before, so ok to push as it is 
now. Above remark is only for the record.

> +        table->field[fld]->set_notnull();
> +        table->field[fld]->store_time(&time, MYSQL_TIMESTAMP_DATETIME);
> +      }
> +      break;
> +    }
> +    case ET_OBH_FIELD_BACKUP_STATE:
> +    {
> +      table->field[fld]->store(val_state, TRUE);
> +      table->field[fld]->set_notnull();
> +      break;
> +    }
>    }
>  
>    /*
>      Update the row.
>    */
>    if ((ret= table->file->ha_update_row(table->record[1],
> table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> +    goto err;
>  
> -  ret= close_backup_progress_table(locking_thd);
> -  DBUG_RETURN(ret);
> +  result= FALSE;
> +
> +err:
> +  if (result && !thd->killed)
> +    sql_print_error("Failed to write to mysql.online_backup.");

I wonder why you don't use internationalized error message and related error 
reporting functions?

> +
> +  if (need_close)
> +    close_performance_schema_table(thd, & open_tables_backup);
> +
> +  thd->time_zone_used= save_time_zone_used;
> +  return result;
>  }
>  
>  /**
> -   report_ob_init()
> +  Write the backup log entry for the backup progress log to a table.
>  
> -   This method inserts a new row in the online_backup table populating it
> -   with the initial values passed. It returns the backup_id of the new row.
> +  This method creates a new row in the backup progress log with the
> +  information provided.
>  
> -   @param int                process_id   The process id of the operation 
> -   @param enum_backup_state  state        The current state of the operation
> -   @param enum_backup_op     operation    The current state of the operation
> -   @param int                error_num    The error number
> -   @param char *             user_comment The user's comment specified in the
> -                                          command (not implemented yet)
> -   @param char *             backup_file  The name of the target file
> -   @param char *             command      The actual command entered
> +  @param[IN]   thd         The current thread
> +  @param[OUT]  backup_id   The new row id for the backup history

This description is confusing. Shouldn't it be more like "the id of the 
backup/restore operation for which we log progress".

> +  @param[IN]   object      The name of the object processed
> +  @param[IN]   start       Start datetime
> +  @param[IN]   stop        Stop datetime
> +  @param[IN]   size        Size value
> +  @param[IN]   progress    Progress (percent)
> +  @param[IN]   error_num   Error number (should be 0 is success)

s/0 is/0 if/

> +  @param[IN]   notes       Misc data from engine
>  
> -   @returns long backup_id  The autoincrement value for the new row.
> -  */
> -ulonglong report_ob_init(int process_id,
> -                    enum_backup_state state,
> -                    enum_backup_op operation,
> -                    int error_num,
> -                    const char *user_comment,
> -                    const char *backup_file,
> -                    const char *command)
> +  @returns TRUE if error.
> +
> +  @todo Add internal error handler to handle errors that occur on
> +        open. See  thd->push_internal_handler(&error_handler).
> +*/
> +bool backup_progress_log_write(THD *thd,
> +                               ulonglong backup_id,
> +                               const char *object,
> +                               time_t start,
> +                               time_t stop,
> +                               longlong size,
> +                               longlong progress,
> +                               int error_num,
> +                               const char *notes)
>  {
> -  ulonglong backup_id= 0;
> -  int ret= 0;                                  // return value
> -  TABLE *table= NULL;                          // table to open
> -  TABLE_LIST tables;                           // List of tables (1 in this case)
> -  char *host= current_thd->security_ctx->host; // host name
> -  char *user= current_thd->security_ctx->user; // user name
> -  Locking_thread_st *locking_thd= NULL;        // The locking thread
> +  TABLE_LIST table_list;
> +  TABLE *table;
> +  bool result= TRUE;
> +  bool need_close= FALSE;
> +  bool need_pop= FALSE;
> +  bool need_rnd_end= FALSE;
> +  Open_tables_state open_tables_backup;
> +  bool save_time_zone_used;
>  
> -  DBUG_ENTER("report_ob_init()");
> +  /*
> +    CSV uses TIME_to_timestamp() internally if table needs to be repaired
> +    which will set thd->time_zone_used
> +  */

This comment was not given before in a similar place...

> +  save_time_zone_used= thd->time_zone_used;
>  
> -  locking_thd= open_backup_progress_table("online_backup", TL_WRITE);
> -  if (!locking_thd)
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> +  bzero(& table_list, sizeof(TABLE_LIST));
> +  table_list.alias= table_list.table_name= BACKUP_PROGRESS_LOG_NAME.str;
> +  table_list.table_name_length= BACKUP_PROGRESS_LOG_NAME.length;
> +
> +  table_list.lock_type= TL_WRITE_CONCURRENT_INSERT;
> +
> +  table_list.db= MYSQL_SCHEMA_NAME.str;
> +  table_list.db_length= MYSQL_SCHEMA_NAME.length;
> +
> +  if (!(table= open_performance_schema_table(thd, & table_list,
> +                                             & open_tables_backup)))
> +    goto err;

Again repeated code. Suggest even more that a helper function is needed.

>  
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> +  need_close= TRUE;
>  
> -  THD *t= table->in_use;
> -  table->in_use= current_thd;
> +  if (table->file->extra(HA_EXTRA_MARK_AS_LOG_TABLE) ||
> +      table->file->ha_rnd_init(0))
> +    goto err;
> +
> +  need_rnd_end= TRUE;
> +
> +  /* Honor next number columns if present */
> +  table->next_number_field= table->found_next_number_field;
>  
>    /*
>      Get defaults for new record.
>    */
>    restore_record(table, s->default_values); 
>  
> +  /* check that all columns exist */
> +  if (table->s->fields < 8)

Use ET_OBP_FIELD_PROG_COUNT here - easier to maintain.

> +    goto err;
> +
>    /*
>      Fill in the data.
>    */
> -  table->field[ET_FIELD_PROCESS_ID]->store(process_id, TRUE);
> -  table->field[ET_FIELD_PROCESS_ID]->set_notnull();
> -  table->field[ET_FIELD_BACKUP_STATE]->store(state, TRUE);
> -  table->field[ET_FIELD_BACKUP_STATE]->set_notnull();
> -  table->field[ET_FIELD_OPER]->store(operation, TRUE);
> -  table->field[ET_FIELD_OPER]->set_notnull();
> -  table->field[ET_FIELD_ERROR_NUM]->store(error_num, TRUE);
> -  table->field[ET_FIELD_ERROR_NUM]->set_notnull();
> +  table->field[ET_OBP_FIELD_BACKUP_ID_FK]->store(backup_id, TRUE);
> +  table->field[ET_OBP_FIELD_BACKUP_ID_FK]->set_notnull();
>  
> -  if (host)
> +  if (object)
>    {
> -    if(table->field[ET_FIELD_HOST_OR_SERVER]->store(host, 
> -       strlen(host), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_HOST_OR_SERVER]->set_notnull();
> +    if (table->field[ET_OBP_FIELD_PROG_OBJECT]->store(object,
> +        strlen(object), system_charset_info))
> +      goto err;
> +    table->field[ET_OBP_FIELD_PROG_OBJECT]->set_notnull();
>    }
>  
> -  if (user)
> +  if (notes)
>    {
> -    if (table->field[ET_FIELD_USERNAME]->store(user,
> -        strlen(user), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_USERNAME]->set_notnull();
> +    if (table->field[ET_OBP_FIELD_PROG_NOTES]->store(notes,
> +        strlen(notes), system_charset_info))
> +      goto err;
> +    table->field[ET_OBP_FIELD_PROG_NOTES]->set_notnull();
>    }
>  
> -  if (user_comment)
> +  if (start)
>    {
> -    if (table->field[ET_FIELD_COMMENT]->store(user_comment,
> -        strlen(user_comment), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_COMMENT]->set_notnull();
> +    MYSQL_TIME time;
> +    my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)start);
> +
> +    table->field[ET_OBP_FIELD_PROG_START_TIME]->set_notnull();
> +    table->field[ET_OBP_FIELD_PROG_START_TIME]->store_time(&time,
> MYSQL_TIMESTAMP_DATETIME);
>    }
>  
> -  if (backup_file)
> +  if (stop)
>    {
> -    if (table->field[ET_FIELD_BACKUP_FILE]->store(backup_file, 
> -        strlen(backup_file), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_BACKUP_FILE]->set_notnull();
> +    MYSQL_TIME time;
> +    my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)stop);
> +
> +    table->field[ET_OBP_FIELD_PROG_STOP_TIME]->set_notnull();
> +    table->field[ET_OBP_FIELD_PROG_STOP_TIME]->store_time(&time,
> MYSQL_TIMESTAMP_DATETIME);
>    }
>  
> -  if (command)
> +  table->field[ET_OBP_FIELD_PROG_SIZE]->store(size, TRUE);
> +  table->field[ET_OBP_FIELD_PROG_SIZE]->set_notnull();
> +  table->field[ET_OBP_FIELD_PROGRESS]->store(progress, TRUE);
> +  table->field[ET_OBP_FIELD_PROGRESS]->set_notnull();
> +  table->field[ET_OBP_FIELD_PROG_ERROR_NUM]->store(error_num, TRUE);
> +  table->field[ET_OBP_FIELD_PROG_ERROR_NUM]->set_notnull();
> +
> +  /* log table entries are not replicated */
> +  if (table->file->ha_write_row(table->record[0]))
> +    goto err;
> +
> +  result= FALSE;
> +
> +err:
> +  if (result && !thd->killed)
> +    sql_print_error("Failed to write to mysql.online_backup_progress.");
> +
> +  if (need_rnd_end)
>    {
> -    if (table->field[ET_FIELD_COMMAND]->store(command,
> -        strlen(command), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_COMMAND]->set_notnull();
> +    table->file->ha_rnd_end();
> +    table->file->ha_release_auto_increment();
>    }
> -  table->in_use= t;
> +  if (need_close)
> +    close_performance_schema_table(thd, & open_tables_backup);
>  
> -  table->next_number_field=table->found_next_number_field;
> +  thd->time_zone_used= save_time_zone_used;
> +  return result;
> +}
>  
> -  /*
> -    Write the row.
> -  */
> -  if ((ret= table->file->ha_write_row(table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> +/**
> +   Find the row in the table that matches the backup_id.
> +
> +   This method locates the row in the online backup table that matches the
> +   backup_id passed.
>  
> +   2param TABLE      table      The open table.

s/2param/@param/

> +   @param ulonglong  backup_id  The id of the row to locate.
> +
> +   @returns 0 = success
> +   @returns 1 = failed to find row
> +  */
> +bool find_backup_history_row(TABLE *table, ulonglong backup_id)
> +{
> +  uchar key[MAX_KEY_LENGTH]; // key buffer for search
>    /*
> -    Get last insert id for row.
> +    Create key to find row. We have to use field->store() to be able to
> +    handle VARCHAR and CHAR fields.
> +    Assumption here is that the two first fields in the table are
> +    'db' and 'name' and the first key is the primary key over the
> +    same fields.

I don't understand this comment. We store backup_id of type ulonglong - why it 
talks about VARCHAR and CHAR? What is 'db' and 'name' in this context?

>    */
> -  backup_id= table->file->insert_id_for_cur_row;
> -  table->file->ha_release_auto_increment();
> +  table->field[ET_OBH_FIELD_BACKUP_ID]->store(backup_id, TRUE);
>  
> -end:
> +  key_copy(key, table->record[0], table->key_info,
> table->key_info->key_length);
> +
> +  if (table->file->index_read_idx_map(table->record[0], 0, key,
> HA_WHOLE_KEY,
> +                                      HA_READ_KEY_EXACT))
> +    return true;
> +
> +  return false;
> +}
> +
> +/**
> +   report_ob_init()
> +
> +   This method inserts a new row in the online_backup table populating it
> +   with the initial values passed. It returns the backup_id of the new row.
> +
> +   @param THD                thd          The current thread class.
> +   @param int                process_id   The process id of the operation 
> +   @param enum_backup_state  state        The current state of the operation
> +   @param enum_backup_op     operation    The current state of the operation
> +   @param int                error_num    The error number
> +   @param char *             user_comment The user's comment specified in the
> +                                          command (not implemented yet)
> +   @param char *             backup_file  The name of the target file
> +   @param char *             command      The actual command entered
> +
> +   @returns long backup_id  The autoincrement value for the new row.
> +  */
> +ulonglong report_ob_init(THD *thd, 
> +                         int process_id,
> +                         enum_backup_state state,
> +                         enum_backup_operation operation,
> +                         int error_num,
> +                         const char *user_comment,
> +                         const char *backup_file,
> +                         const char *command)
> +{ 
> +  ulonglong backup_id= 0;
> +  int ret= 0;                                  // return value
> +  DBUG_ENTER("report_ob_init()");
>  
> -  ret= close_backup_progress_table(locking_thd);
> +  ret= backup_history_log_write(thd, &backup_id, process_id, state, operation,
> +                                error_num, user_comment, backup_file, command);
>    /*
>      Record progress update.
>    */
>    String str;
>    get_state_string(state, &str);
> -  report_ob_progress(backup_id, "backup kernel", 0, 
> +  report_ob_progress(thd, backup_id, "backup kernel", 0, 
>                       0, 0, 0, 0, str.c_ptr());
>    DBUG_RETURN(backup_id);
>  }
> @@ -505,6 +640,7 @@ end:
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the binlog values.
>  
> +   @param THD                  thd          The current thread class.
>     @param ulonglong            backup_id    The id of the row to locate.
>     @param int                  backup_pos   The id of the row to locate.
>     @param char *               binlog_file  The filename of the binlog.
> @@ -512,62 +648,18 @@ end:
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_binlog_info(ulonglong backup_id,
> +int report_ob_binlog_info(THD *thd,
> +                          ulonglong backup_id,
>                            int binlog_pos,
>                            const char *binlog_file)
>  {
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
>    int ret= 0;                           // return value
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> -
>    DBUG_ENTER("report_ob_binlog_info()");
>  
> -  locking_thd= open_backup_progress_table("online_backup", TL_WRITE);
> -  if (!locking_thd)
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> -
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> -
> -  if (find_online_backup_row(table, backup_id))
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(1);
> -  }
> -
> -  store_record(table, record[1]);
> -
> -  /*
> -    Fill in the data.
> -  */
> -  table->field[ET_FIELD_BINLOG_POS]->store(binlog_pos, TRUE);
> -  table->field[ET_FIELD_BINLOG_POS]->set_notnull();
> -
> -  THD *t= table->in_use;
> -  table->in_use= current_thd;
> -
> -  if (binlog_file)
> -  {
> -    if(table->field[ET_FIELD_BINLOG_FILE]->store(binlog_file, 
> -       strlen(binlog_file), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_BINLOG_FILE]->set_notnull();
> -  }
> -  table->in_use= t;
> -
> -  /*
> -    Update the row.
> -  */
> -  if ((ret= table->file->ha_update_row(table->record[1],
> table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> -
> -end:
> -
> -  ret= close_backup_progress_table(locking_thd);
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_BINLOG_POS, 
> +                                 binlog_pos, 0, 0, 0);
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_BINLOG_FILE, 
> +                                 0, 0, binlog_file, 0);
>    DBUG_RETURN(ret);
>  }
>  
> @@ -577,20 +669,23 @@ end:
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the error number value.
>  
> +   @param THD        thd        The current thread class.
>     @param ulonglong  backup_id  The id of the row to locate.
>     @param int        error_num  New error number.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_error(ulonglong backup_id,
> +int report_ob_error(THD *thd,
> +                    ulonglong backup_id,
>                      int error_num)
>  {
>    int ret= 0;  // return value
> -
>    DBUG_ENTER("report_ob_error()");
> -  update_online_backup_int_field(backup_id, "online_backup", 
> -                                 ET_FIELD_ERROR_NUM, error_num);
> +
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_ERROR_NUM, 
> +                                 error_num, 0, 0, 0);
> +
>    DBUG_RETURN(ret);
>  }
>  
> @@ -600,20 +695,23 @@ int report_ob_error(ulonglong backup_id,
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the number of objects value.
>  
> -   @param ulonglong  backup_id   The id of the row to locate.
> +   @param THD        thd         The current thread class.
> +   @param ulonglong  backup_id    The id of the row to locate.
>     @param int        num_objects  New error number.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row

Wrong doxygen format. It should look as follows:

   @retval 0  success
   @retval 1  failed to find row

The same problem occurs elsewhere in the patch.

>    */
> -int report_ob_num_objects(ulonglong backup_id,
> +int report_ob_num_objects(THD *thd,
> +                          ulonglong backup_id,
>                            int num_objects)
>  {
>    int ret= 0;  // return value
> -
>    DBUG_ENTER("report_ob_num_objects()");
> -  update_online_backup_int_field(backup_id, "online_backup",
> -                                 ET_FIELD_NUM_OBJ, num_objects);
> +
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_NUM_OBJ, 
> +                                 num_objects, 0, 0, 0);
> +
>    DBUG_RETURN(ret);
>  }
>  
> @@ -623,20 +721,23 @@ int report_ob_num_objects(ulonglong back
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the size value.
>  
> +   @param THD        thd        The current thread class.
>     @param ulonglong  backup_id  The id of the row to locate.
>     @param int        size       New size value.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_size(ulonglong backup_id,
> +int report_ob_size(THD *thd,
> +                   ulonglong backup_id,
>                     longlong size)
>  {
>    int ret= 0;  // return value
> -
>    DBUG_ENTER("report_ob_size()");
> -  update_online_backup_int_field(backup_id, "online_backup",
> -                                 ET_FIELD_TOTAL_BYTES, size);
> +
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_TOTAL_BYTES, 
> +                                 size, 0, 0, 0);
> +
>    DBUG_RETURN(ret);
>  }
>  
> @@ -646,6 +747,7 @@ int report_ob_size(ulonglong backup_id,
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the start/stop values.
>  
> +   @param THD        thd        The current thread class.
>     @param ulonglong  backup_id  The id of the row to locate.
>     @param my_time_t  start      Start datetime.
>     @param my_time_t  stop       Stop datetime.
> @@ -653,19 +755,22 @@ int report_ob_size(ulonglong backup_id,
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_time(ulonglong backup_id,
> +int report_ob_time(THD *thd,
> +                   ulonglong backup_id,
>                     time_t start,
>                     time_t stop)
>  {
>    int ret= 0;  // return value
> -
>    DBUG_ENTER("report_ob_time()");
> +
>    if (start)
> -    update_online_backup_datetime_field(backup_id, "online_backup",
> -                                        ET_FIELD_START_TIME, start);
> +    ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_START_TIME, 
> +                                   0, start, 0, 0);
> +
>    if (stop)
> -    update_online_backup_datetime_field(backup_id, "online_backup",
> -                                        ET_FIELD_STOP_TIME, stop);
> +    ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_STOP_TIME, 
> +                                   0, stop, 0, 0);
> +
>    DBUG_RETURN(ret);
>  }
>  
> @@ -675,21 +780,24 @@ int report_ob_time(ulonglong backup_id,
>     This method updates the validity point time for the backup operation
>     identified by backup_id.
>  
> +   @param THD        thd        The current thread class.
>     @param ulonglong  backup_id  The id of the row to locate.
>     @param my_time_t  vp_time    Validity point datetime.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_vp_time(ulonglong backup_id,
> +int report_ob_vp_time(THD *thd,
> +                      ulonglong backup_id,
>                        time_t vp_time)
>  {
>    int ret= 0;  // return value
> -
>    DBUG_ENTER("report_ob_vp_time()");
> +
>    if (vp_time)
> -    update_online_backup_datetime_field(backup_id, "online_backup",
> -                                        ET_FIELD_VP, vp_time);
> +    ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_VP, 
> +                                   0, vp_time, 0, 0);
> +
>    DBUG_RETURN(ret);
>  }
>  
> @@ -700,70 +808,23 @@ int report_ob_vp_time(ulonglong backup_i
>     identified by backup_id. This method appends to the those listed in the
>     table for the backup_id.
>  
> +   @param THD        thd          The current thread class.
>     @param ulonglong  backup_id    The id of the row to locate.
>     @param char *     egnine_name  The name of the engine to add.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_engines(ulonglong backup_id,
> +int report_ob_engines(THD *thd,
> +                      ulonglong backup_id,
>                        const char *engine_name)
>  {
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  int ret= 0;                           // return value
> -  String str;                           // engines string
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> -
> -  DBUG_ENTER("report_ob_engines()");
> -
> -  locking_thd= open_backup_progress_table("online_backup", TL_WRITE);
> -  if (!locking_thd)
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> -
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> -
> -  if (find_online_backup_row(table, backup_id))
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(1);
> -  }
> -
> -  store_record(table, record[1]);
> -
> -  /*
> -    Fill in the data.
> -  */
> -  THD *t= table->in_use;
> -  table->in_use= current_thd;
> -
> -  str.length(0);
> -  table->field[ET_FIELD_ENGINES]->val_str(&str);
> -  if (str.length() > 0)
> -    str.append(", ");
> -  str.append(engine_name);
> -  if (str.length() > 0)
> -  {
> -    if(table->field[ET_FIELD_ENGINES]->store(str.c_ptr(), 
> -       str.length(), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_ENGINES]->set_notnull();
> -  }
> -  table->in_use= t;
> -
> -  /*
> -    Update the row.
> -  */
> -  if ((ret= table->file->ha_update_row(table->record[1],
> table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> +  int ret= 0;  // return value
> +  DBUG_ENTER("report_ob_vp_time()");

Wrong function name in DBUG_ENTER. Please check it also elsewhere.

>  
> -end:
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_ENGINES, 
> +                                 0, 0, engine_name, 0);
>  
> -  ret= close_backup_progress_table(locking_thd);
>    DBUG_RETURN(ret);
>  }
>  
> @@ -773,26 +834,28 @@ end:
>     This method locates the row in the online backup table that matches the
>     backup_id passed and updates the state value.
>  
> +   @param THD                thd        The current thread class.
>     @param ulonglong          backup_id  The id of the row to locate.
>     @param enum_backup_state  state      New state value.
>  
>     @returns 0 = success
>     @returns 1 = failed to find row
>    */
> -int report_ob_state(ulonglong backup_id,
> +int report_ob_state(THD *thd, 
> +                    ulonglong backup_id,
>                      enum_backup_state state)
>  {
>    int ret= 0;  // return value
>    String str;
> -
>    DBUG_ENTER("report_ob_state()");
> -  update_online_backup_int_field(backup_id, "online_backup", 
> -                                 ET_FIELD_BACKUP_STATE, state);
> +
> +  ret= backup_history_log_update(thd, backup_id, ET_OBH_FIELD_BACKUP_STATE, 
> +                                 0, 0, 0, state);
>    /*
>      Record progress update.
>    */
>    get_state_string(state, &str);
> -  report_ob_progress(backup_id, "backup kernel", 0, 
> +  report_ob_progress(thd, backup_id, "backup kernel", 0, 
>                       0, 0, 0, 0, str.c_ptr());
>  
>    DBUG_RETURN(ret);
> @@ -805,6 +868,7 @@ int report_ob_state(ulonglong backup_id,
>     the values passed. This method is used to insert progress information during
>     the backup operation.
>  
> +   @param THD        thd        The current thread class.
>     @param ulonglong  backup_id  The id of the master table row.
>     @param char *     object     The name of the object processed.
>     @param my_time_t  start      Start datetime.
> @@ -817,7 +881,8 @@ int report_ob_state(ulonglong backup_id,
>     @returns 0 = success
>     @returns 1 = failed to write row
>    */
> -int report_ob_progress(ulonglong backup_id,
> +int report_ob_progress(THD *thd,
> +                       ulonglong backup_id,
>                         const char *object,
>                         time_t start,
>                         time_t stop,

Do you still need this function given that there is backup_progress_log_write() 
doing basically the same thing?

> @@ -827,177 +892,13 @@ int report_ob_progress(ulonglong backup_
>                         const char *notes)
>  {
>    int ret= 0;                           // return value
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> -
>    DBUG_ENTER("report_ob_progress()");
>  
> -  locking_thd= open_backup_progress_table("online_backup_progress", TL_WRITE);
> -  if (!locking_thd)
> -  {
> -    ret= close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> -
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> -
> -  THD *t= table->in_use;
> -  table->in_use= current_thd;
> -
> -  /*
> -    Get defaults for new record.
> -  */
> -  restore_record(table, s->default_values); 
> -
> -  /*
> -    Fill in the data.
> -  */
> -  table->field[ET_FIELD_BACKUP_ID_FK]->store(backup_id, TRUE);
> -  table->field[ET_FIELD_BACKUP_ID_FK]->set_notnull();
> -
> -  if (object)
> -  {
> -    if (table->field[ET_FIELD_PROG_OBJECT]->store(object,
> -        strlen(object), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_PROG_OBJECT]->set_notnull();
> -  }
> -
> -  if (notes)
> -  {
> -    if (table->field[ET_FIELD_PROG_NOTES]->store(notes,
> -        strlen(notes), system_charset_info))
> -      goto end;
> -    table->field[ET_FIELD_PROG_NOTES]->set_notnull();
> -  }
> -  table->in_use= t;
> -
> -  if (start)
> -  {
> -    MYSQL_TIME time;
> -    my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)start);
> -
> -    table->field[ET_FIELD_PROG_START_TIME]->set_notnull();
> -    table->field[ET_FIELD_PROG_START_TIME]->store_time(&time,
> MYSQL_TIMESTAMP_DATETIME);
> -  }
> -
> -  if (stop)
> -  {
> -    MYSQL_TIME time;
> -    my_tz_OFFSET0->gmt_sec_to_TIME(&time, (my_time_t)stop);
> -
> -    table->field[ET_FIELD_PROG_STOP_TIME]->set_notnull();
> -    table->field[ET_FIELD_PROG_STOP_TIME]->store_time(&time,
> MYSQL_TIMESTAMP_DATETIME);
> -  }
> +  ret= backup_progress_log_write(thd, backup_id, object, start, stop, 
> +                                 size, progress, error_num, notes);
>  
> -  table->field[ET_FIELD_PROG_SIZE]->store(size, TRUE);
> -  table->field[ET_FIELD_PROG_SIZE]->set_notnull();
> -  table->field[ET_FIELD_PROGRESS]->store(progress, TRUE);
> -  table->field[ET_FIELD_PROGRESS]->set_notnull();
> -  table->field[ET_FIELD_PROG_ERROR_NUM]->store(error_num, TRUE);
> -  table->field[ET_FIELD_PROG_ERROR_NUM]->set_notnull();
> -
> -  /*
> -    Write the row.
> -  */
> -  if ((ret= table->file->ha_write_row(table->record[0])))
> -    table->file->print_error(ret, MYF(0));
> -
> -end:
> -
> -  ret= close_backup_progress_table(locking_thd);
>    DBUG_RETURN(ret);
>  }
>  
> -/**
> -   Sums the sizes for the row that matches the backup_id.
> -
> -   This method sums the size entries from the online backup progress rows
> -   for the backup operation identified by backup_id.
> -
> -   @param ulonglong          backup_id  The id of the row to locate.
> -
> -   @returns  ulonglong  Total size of all backup progress rows
> -  */
> -ulonglong sum_progress_rows(ulonglong backup_id)
> -{
> -  int last_read_res;                    // result of last read
> -  TABLE *table= NULL;                   // table to open
> -  TABLE_LIST tables;                    // List of tables (1 in this case)
> -  ulonglong size= 0;                    // total size
> -  handler *hdl;                         // handler pointer
> -  Locking_thread_st *locking_thd= NULL; // The locking thread
> -
> -  DBUG_ENTER("sum_progress_rows()");
> -
> -  locking_thd= open_backup_progress_table("online_backup_progress", TL_READ);
> -  if (!locking_thd)
> -  {
> -    close_backup_progress_table(locking_thd);
> -    DBUG_RETURN(0);
> -  }
> -
> -  table= locking_thd->tables_in_backup->table;
> -  table->use_all_columns();
> -
> -  hdl= table->file;
> -  last_read_res= hdl->ha_rnd_init(1);
> -  THD *t= table->in_use;
> -  table->in_use= current_thd;
> -  while (!hdl->rnd_next(table->record[0]))
> -    if ((table->field[ET_FIELD_PROGRESS]->val_int() == 100) &&
> -        (table->field[ET_FIELD_PROG_ERROR_NUM]->val_int() == 0) &&
> -        ((ulonglong)table->field[ET_FIELD_BACKUP_ID_FK]->val_int() ==
> backup_id))
> -      size+= table->field[ET_FIELD_PROG_SIZE]->val_int();
> -  table->in_use= t;
> -
> -  hdl->ha_rnd_end();
> -
> -  close_backup_progress_table(locking_thd);
> -  DBUG_RETURN(size);
> -}
> -
> -/**
> -   Print summary for the row that matches the backup_id.
> -
> -   This method prints the summary information for the backup operation
> -   identified by backup_id.
>  
> -   @param ulonglong  backup_id  The id of the row to locate.
> -
> -   @returns 0 = success
> -   @returns 1 = failed to find row
> -  */
> -int print_backup_summary(THD *thd, ulonglong backup_id)
> -{
> -  Protocol *protocol= thd->protocol;    // client comms
> -  List<Item> field_list;                // list of fields to send
> -  String     op_str;                    // operations string
> -  int ret= 0;                           // return value
> -  char buf[255];                        // buffer for summary information
> -  String str;
> -
> -  DBUG_ENTER("print_backup_summary()");
> -
> -  /*
> -    Send field list.
> -  */
> -  op_str.length(0);
> -  op_str.append("backup_id");
> -  field_list.push_back(new Item_empty_string(op_str.c_ptr(), op_str.length()));
> -  protocol->send_fields(&field_list, Protocol::SEND_NUM_ROWS |
> Protocol::SEND_EOF);
> -
> -  /*
> -    Send field data.
> -  */
> -  protocol->prepare_for_resend();
> -  llstr(backup_id,buf);
> -  protocol->store(buf, system_charset_info);
> -  protocol->write();
> -
> -  my_eof(thd);
> -  DBUG_RETURN(ret);
> -}
>  
==================================================================================
> diff -Nrup a/sql/backup/backup_progress.h b/sql/backup/backup_progress.h
> --- a/sql/backup/backup_progress.h	2008-01-31 12:23:30 -05:00
> +++ b/sql/backup/backup_progress.h	2008-03-19 12:56:44 -04:00
> @@ -29,43 +29,43 @@
>  /**
>     List of fields for online backup table.
>    */
> -enum enum_ob_table_field
> +enum enum_backup_history_table_field
>  {
> -  ET_FIELD_BACKUP_ID = 0, /* start from 0 to correspond with field array */
> -  ET_FIELD_PROCESS_ID,
> -  ET_FIELD_BINLOG_POS,
> -  ET_FIELD_BINLOG_FILE,
> -  ET_FIELD_BACKUP_STATE,
> -  ET_FIELD_OPER,
> -  ET_FIELD_ERROR_NUM,
> -  ET_FIELD_NUM_OBJ,
> -  ET_FIELD_TOTAL_BYTES,
> -  ET_FIELD_VP,
> -  ET_FIELD_START_TIME,
> -  ET_FIELD_STOP_TIME,
> -  ET_FIELD_HOST_OR_SERVER,
> -  ET_FIELD_USERNAME,
> -  ET_FIELD_BACKUP_FILE,
> -  ET_FIELD_COMMENT,
> -  ET_FIELD_COMMAND,
> -  ET_FIELD_ENGINES,
> -  ET_FIELD_COUNT /* a cool trick to count the number of fields :) */
> +  ET_OBH_FIELD_BACKUP_ID = 0, /* start from 0 to correspond with field array */
> +  ET_OBH_FIELD_PROCESS_ID,
> +  ET_OBH_FIELD_BINLOG_POS,
> +  ET_OBH_FIELD_BINLOG_FILE,
> +  ET_OBH_FIELD_BACKUP_STATE,
> +  ET_OBH_FIELD_OPER,
> +  ET_OBH_FIELD_ERROR_NUM,
> +  ET_OBH_FIELD_NUM_OBJ,
> +  ET_OBH_FIELD_TOTAL_BYTES,
> +  ET_OBH_FIELD_VP,
> +  ET_OBH_FIELD_START_TIME,
> +  ET_OBH_FIELD_STOP_TIME,
> +  ET_OBH_FIELD_HOST_OR_SERVER,
> +  ET_OBH_FIELD_USERNAME,
> +  ET_OBH_FIELD_BACKUP_FILE,
> +  ET_OBH_FIELD_COMMENT,
> +  ET_OBH_FIELD_COMMAND,
> +  ET_OBH_FIELD_ENGINES,
> +  ET_OBH_FIELD_COUNT /* a cool trick to count the number of fields :) */
>  };
>  
>  /**
>     List of fields for online backup progress table.
>    */
> -enum enum_ob_progress_table_field
> +enum enum_backup_progress_table_field
>  {
> -  ET_FIELD_BACKUP_ID_FK = 0, /* start from 0 to correspond with field array */
> -  ET_FIELD_PROG_OBJECT,
> -  ET_FIELD_PROG_START_TIME,
> -  ET_FIELD_PROG_STOP_TIME,
> -  ET_FIELD_PROG_SIZE,
> -  ET_FIELD_PROGRESS,
> -  ET_FIELD_PROG_ERROR_NUM,
> -  ET_FIELD_PROG_NOTES,
> -  ET_FIELD_PROG_COUNT /* a cool trick to count the number of fields :) */
> +  ET_OBP_FIELD_BACKUP_ID_FK = 0, /* start from 0 to correspond with field array */
> +  ET_OBP_FIELD_PROG_OBJECT,
> +  ET_OBP_FIELD_PROG_START_TIME,
> +  ET_OBP_FIELD_PROG_STOP_TIME,
> +  ET_OBP_FIELD_PROG_SIZE,
> +  ET_OBP_FIELD_PROGRESS,
> +  ET_OBP_FIELD_PROG_ERROR_NUM,
> +  ET_OBP_FIELD_PROG_NOTES,
> +  ET_OBP_FIELD_PROG_COUNT /* a cool trick to count the number of fields :) */
>  };
>  
>  /**
> @@ -73,7 +73,8 @@ enum enum_ob_progress_table_field
>    */
>  enum enum_backup_state
>  {
> -  BUP_COMPLETE = 1,
> +  BUP_UNKNOWN = 0,
> +  BUP_COMPLETE,
>    BUP_STARTING,
>    BUP_VALIDITY_POINT,
>    BUP_RUNNING,
> @@ -84,7 +85,7 @@ enum enum_backup_state
>  /**
>     List of operations for online backup table.
>    */
> -enum enum_backup_op
> +enum enum_backup_operation
>  {
>    OP_BACKUP = 1,
>    OP_RESTORE,
> @@ -98,23 +99,54 @@ enum enum_backup_op
>  */
>  my_bool check_ob_progress_tables(THD *thd);
>  
> +bool backup_history_log_write(THD *thd, 
> +                              ulonglong *backup_id,
> +                              int process_id,
> +                              enum_backup_state state,
> +                              enum_backup_operation operation,
> +                              int error_num,
> +                              const char *user_comment,
> +                              const char *backup_file,
> +                              const char *command);
> +
> +bool find_backup_history_row(TABLE *table, ulonglong backup_id);
> +
> +bool backup_history_log_update(THD *thd, 
> +                               ulonglong backup_id,
> +                               enum_backup_history_table_field fld,
> +                               ulonglong val_long,
> +                               time_t val_time,
> +                               const char *val_str,
> +                               int val_state);
> +
> +bool backup_progress_log_write(THD *thd,
> +                               ulonglong backup_id,
> +                               const char *object,
> +                               time_t start,
> +                               time_t stop,
> +                               longlong size,
> +                               longlong progress,
> +                               int error_num,
> +                               const char *notes);
>  /*
>    This method inserts a new row in the online_backup table populating it with
>    the initial values passed. It returns the backup_id of the new row.
>  */
> -ulonglong report_ob_init(int process_id,
> -                    enum_backup_state state,
> -                    enum_backup_op operation,
> -                    int error_num,
> -                    const char *user_comment,
> -                    const char *backup_file,
> -                    const char *command);
> +ulonglong report_ob_init(THD *thd,
> +                         int process_id,
> +                         enum_backup_state state,
> +                         enum_backup_operation operation,
> +                         int error_num,
> +                         const char *user_comment,
> +                         const char *backup_file,
> +                         const char *command);
>  
>  /*
>    This method updates the binary log information for the backup operation
>    identified by backup_id.
>  */
> -int report_ob_binlog_info(ulonglong backup_id,
> +int report_ob_binlog_info(THD *thd,
> +                          ulonglong backup_id,
>                            int binlog_pos,
>                            const char *binlog_file);
>  
> @@ -122,28 +154,32 @@ int report_ob_binlog_info(ulonglong back
>    This method updates the error number for the backup operation identified by
>    backup_id.
>  */
> -int report_ob_error(ulonglong backup_id,
> +int report_ob_error(THD *thd,
> +                    ulonglong backup_id,
>                      int error_num);
>  
>  /*
>    This method updates the state for the backup operation identified by
>    backup_id.
>  */
> -int report_ob_state(ulonglong backup_id,
> +int report_ob_state(THD *thd,
> +                    ulonglong backup_id,
>                      enum_backup_state state);
>  
>  /*
>    This method updates the number of objects for the backup operation identified by
>    backup_id.
>  */
> -int report_ob_num_objects(ulonglong backup_id,
> +int report_ob_num_objects(THD *thd,
> +                          ulonglong backup_id,
>                            int num_objects);
>  
>  /*
>    This method updates the size for the backup operation identified by
>    backup_id.
>  */
> -int report_ob_size(ulonglong backup_id,
> +int report_ob_size(THD *thd,
> +                   ulonglong backup_id,
>                     longlong size);
>  
>  /*
> @@ -154,7 +190,8 @@ int report_ob_size(ulonglong backup_id,
>    values provided so that it can be called once for start and once again later 
>    for stop).
>  */
> -int report_ob_time(ulonglong backup_id,
> +int report_ob_time(THD *thd,
> +                   ulonglong backup_id,
>                     time_t start,
>                     time_t stop);
>  
> @@ -162,7 +199,8 @@ int report_ob_time(ulonglong backup_id,
>    This method updates the validity point time for the backup operation
>    identified by backup_id.
>  */
> -int report_ob_vp_time(ulonglong backup_id,
> +int report_ob_vp_time(THD *thd,
> +                      ulonglong backup_id,
>                        time_t vp_time);
>  
>  /*
> @@ -170,13 +208,15 @@ int report_ob_vp_time(ulonglong backup_i
>    identified by backup_id. This method appends to the those listed in the
>    table for the backup_id.
>  */
> -int report_ob_engines(ulonglong backup_id,
> +int report_ob_engines(THD *thd,
> +                      ulonglong backup_id,
>                        const char *engine_name);
>  
>  /*
>    This method inserts a new row in the progress table.
>  */
> -int report_ob_progress(ulonglong backup_id,
> +int report_ob_progress(THD *thd,
> +                       ulonglong backup_id,
>                         const char *object,
>                         time_t start,
>                         time_t stop,
> @@ -184,18 +224,6 @@ int report_ob_progress(ulonglong backup_
>                         longlong progress,
>                         int error_num,
>                         const char *notes);
> -
> -/*
> -  This method sums the size entries from the online backup progress rows
> -  for the backup operation identified by backup_id.
> -*/
> -ulonglong sum_progress_rows(ulonglong backup_id);
> -
> -/*
> -  This method prints the summary information for the backup operation
> -  identified by backup_id.
> -*/
> -int print_backup_summary(THD *thd, ulonglong backup_id);
>  
>  #endif
>  
==================================================================================
> diff -Nrup a/sql/backup/be_snapshot.cc b/sql/backup/be_snapshot.cc
> --- a/sql/backup/be_snapshot.cc	2007-11-29 23:22:05 -05:00
> +++ b/sql/backup/be_snapshot.cc	2008-03-19 12:56:44 -04:00
> @@ -121,6 +121,7 @@ result_t Backup::get_data(Buffer &buf)
>    if (locking_thd->lock_state == LOCK_SIGNAL)
>    {
>      locking_thd->lock_state= LOCK_DONE; // set lock done so destructor won't
> wait
> +    ha_autocommit_or_rollback(locking_thd->m_thd, 0);
>      end_active_trans(locking_thd->m_thd);
>      close_thread_tables(locking_thd->m_thd);
>    }
==================================================================================
> diff -Nrup a/sql/backup/data_backup.cc b/sql/backup/data_backup.cc
> --- a/sql/backup/data_backup.cc	2008-03-12 08:20:53 -04:00
> +++ b/sql/backup/data_backup.cc	2008-03-19 12:56:45 -04:00
> @@ -491,7 +491,7 @@ int write_table_data(THD* thd, Backup_in
>      /*
>        Record the backup id and name for this driver.
>      */
> -    report_ob_engines(info.backup_prog_id, p->m_name);
> +    report_ob_engines(thd, info.backup_prog_id, p->m_name);
>  
>      if (!p || !p->is_valid())
>      {
> @@ -600,7 +600,7 @@ int write_table_data(THD* thd, Backup_in
>      // VP creation
>      DBUG_PRINT("backup_data",("-- SYNC PHASE --"));
>  
> -    report_ob_state(info.backup_prog_id, BUP_VALIDITY_POINT);
> +    report_ob_state(thd, info.backup_prog_id, BUP_VALIDITY_POINT);
>      /*
>        Block commits.
>  
> @@ -654,12 +654,12 @@ int write_table_data(THD* thd, Backup_in
>      */
>      BACKUP_BREAKPOINT("bp_vp_state");
>      info.save_vp_time(vp_time);
> -    report_ob_vp_time(info.backup_prog_id, vp_time);
> -    report_ob_state(info.backup_prog_id, BUP_RUNNING);
> +    report_ob_vp_time(thd, info.backup_prog_id, vp_time);
> +    report_ob_state(thd, info.backup_prog_id, BUP_RUNNING);
>      BACKUP_BREAKPOINT("bp_running_state");
>  
>      if (mysql_bin_log.is_open())
> -      report_ob_binlog_info(info.backup_prog_id, 
> +      report_ob_binlog_info(thd, info.backup_prog_id, 
>                              info.binlog_pos.pos, info.binlog_pos.file);
>  
>      // get final data from drivers
> @@ -1352,7 +1352,7 @@ namespace backup {
>  /**
>    Read backup image data from a backup stream and forward it to restore drivers.
>   */
> -int restore_table_data(THD*, Restore_info &info, IStream &s)
> +int restore_table_data(THD* thd, Restore_info &info, IStream &s)
>  {
>    DBUG_ENTER("restore::restore_table_data");
>  
> @@ -1410,7 +1410,7 @@ int restore_table_data(THD*, Restore_inf
>      /*
>        Record the name for this driver.
>      */
> -    report_ob_engines(info.backup_prog_id, snap->name());
> +    report_ob_engines(thd, info.backup_prog_id, snap->name());
>    }
>  
>    {
==================================================================================
> diff -Nrup a/sql/backup/kernel.cc b/sql/backup/kernel.cc
> --- a/sql/backup/kernel.cc	2008-02-29 09:37:41 -05:00
> +++ b/sql/backup/kernel.cc	2008-03-19 12:56:46 -04:00
> @@ -149,6 +149,9 @@ execute_backup_command(THD *thd, LEX *le
>    {
>      backup::IStream *stream= open_for_read(*loc);
>  
> +    backup_prog_id= report_ob_init(thd, thd->id, BUP_STARTING, OP_RESTORE, 
> +                                   0, "", lex->backup_dir.str, thd->query);
> +
>      if (!stream)
>      {
>        my_error(ER_BACKUP_READ_LOC,MYF(0),loc->describe());
> @@ -164,9 +167,7 @@ execute_backup_command(THD *thd, LEX *le
>  
>        start= my_time(0);
>        
> -      backup_prog_id= report_ob_init(thd->id, BUP_STARTING, OP_RESTORE, 
> -                                     0, "", lex->backup_dir.str, thd->query);
> -      report_ob_time(backup_prog_id, start, 0);
> +      report_ob_time(thd, backup_prog_id, start, 0);
>        BACKUP_BREAKPOINT("bp_starting_state");
>  
>        Restore_info info(thd,*stream);
> @@ -182,7 +183,7 @@ execute_backup_command(THD *thd, LEX *le
>        info.report_error(log_level::INFO,ER_BACKUP_RESTORE_START);
>        info.save_start_time(start);
>  
> -      report_ob_state(backup_prog_id, BUP_RUNNING);
> +      report_ob_state(thd, backup_prog_id, BUP_RUNNING);
>        BACKUP_BREAKPOINT("bp_running_state");
>  
>        /*
> @@ -217,10 +218,10 @@ execute_backup_command(THD *thd, LEX *le
>          goto restore_error;
>        }
>  
> -      report_ob_num_objects(backup_prog_id, info.table_count);
> -      report_ob_size(backup_prog_id, info.data_size);
> -      report_ob_time(backup_prog_id, 0, stop);
> -      report_ob_state(backup_prog_id, BUP_COMPLETE);
> +      report_ob_num_objects(thd, backup_prog_id, info.table_count);
> +      report_ob_size(thd, backup_prog_id, info.data_size);
> +      report_ob_time(thd, backup_prog_id, 0, stop);
> +      report_ob_state(thd, backup_prog_id, BUP_COMPLETE);
>        BACKUP_BREAKPOINT("bp_complete_state");
>  
>        info.report_error(log_level::INFO,ER_BACKUP_RESTORE_DONE);
> @@ -233,12 +234,12 @@ execute_backup_command(THD *thd, LEX *le
>  
>      res= res ? res : ERROR;
>  
> -    report_ob_error(backup_prog_id, res);
> +    report_ob_error(thd, backup_prog_id, res);
>      
>      if (stop)
> -      report_ob_time(backup_prog_id, 0, stop);
> +      report_ob_time(thd, backup_prog_id, 0, stop);
>  
> -    report_ob_state(backup_prog_id, BUP_ERRORS);
> +    report_ob_state(thd, backup_prog_id, BUP_ERRORS);
>      BACKUP_BREAKPOINT("bp_error_state");
>     
>     finish_restore:
> @@ -261,6 +262,9 @@ execute_backup_command(THD *thd, LEX *le
>      bool remove_location= FALSE; 
>      backup::OStream *stream= open_for_write(*loc);
>  
> +    backup_prog_id= report_ob_init(thd, thd->id, BUP_STARTING, OP_BACKUP,
> +                                   0, "", lex->backup_dir.str, thd->query);
> +
>      if (!stream)
>      {
>        my_error(ER_BACKUP_WRITE_LOC,MYF(0),loc->describe());
> @@ -283,9 +287,7 @@ execute_backup_command(THD *thd, LEX *le
>  
>        Backup_info info(thd);
>  
> -      backup_prog_id= report_ob_init(thd->id, BUP_STARTING, OP_BACKUP,
> -                                     0, "", lex->backup_dir.str, thd->query);
> -      report_ob_time(backup_prog_id, start, 0);
> +      report_ob_time(thd, backup_prog_id, start, 0);
>        BACKUP_BREAKPOINT("bp_starting_state");
>  
>        info.backup_prog_id= backup_prog_id;
> @@ -295,7 +297,7 @@ execute_backup_command(THD *thd, LEX *le
>  
>        info.report_error(log_level::INFO,ER_BACKUP_BACKUP_START);
>        info.save_start_time(start);
> -      report_ob_state(backup_prog_id, BUP_RUNNING);
> +      report_ob_state(thd, backup_prog_id, BUP_RUNNING);
>        BACKUP_BREAKPOINT("bp_running_state");
>  
>        info.save_errors();
> @@ -320,7 +322,7 @@ execute_backup_command(THD *thd, LEX *le
>          goto backup_error;
>        }
>  
> -      report_ob_num_objects(backup_prog_id, info.table_count);
> +      report_ob_num_objects(thd, backup_prog_id, info.table_count);
>  
>        if (check_info(thd,info))
>        {
> @@ -350,9 +352,9 @@ execute_backup_command(THD *thd, LEX *le
>          goto backup_error;
>        }
>  
> -      report_ob_size(info.backup_prog_id, info.data_size);
> -      report_ob_time(info.backup_prog_id, 0, stop);
> -      report_ob_state(info.backup_prog_id, BUP_COMPLETE);
> +      report_ob_size(thd, info.backup_prog_id, info.data_size);
> +      report_ob_time(thd, info.backup_prog_id, 0, stop);
> +      report_ob_state(thd, info.backup_prog_id, BUP_COMPLETE);
>        BACKUP_BREAKPOINT("bp_complete_state");
>  
>        info.report_error(log_level::INFO,ER_BACKUP_BACKUP_DONE);
> @@ -366,11 +368,11 @@ execute_backup_command(THD *thd, LEX *le
>  
>      res= res ? res : ERROR;
>  
> -    report_ob_error(backup_prog_id, res);
> -    report_ob_state(backup_prog_id, BUP_ERRORS);
> +    report_ob_error(thd, backup_prog_id, res);
> +    report_ob_state(thd, backup_prog_id, BUP_ERRORS);
>  
>      if (stop)
> -      report_ob_time(backup_prog_id, 0, stop);
> +      report_ob_time(thd, backup_prog_id, 0, stop);
>  
>      /*
>        If the output stream was opened, a file or other system resource
==================================================================================
> diff -Nrup a/sql/table.cc b/sql/table.cc
> --- a/sql/table.cc	2008-02-25 11:05:59 -05:00
> +++ b/sql/table.cc	2008-03-19 12:56:42 -04:00
> @@ -33,6 +33,9 @@ LEX_STRING GENERAL_LOG_NAME= {C_STRING_W
>  /* SLOW_LOG name */
>  LEX_STRING SLOW_LOG_NAME= {C_STRING_WITH_LEN("slow_log")};
>  
> +extern LEX_STRING BACKUP_HISTORY_LOG_NAME;
> +extern LEX_STRING BACKUP_PROGRESS_LOG_NAME;
> +
>  	/* Functions defined in this file */
>  
>  void open_table_error(TABLE_SHARE *share, int error, int db_errno,
> @@ -241,6 +244,22 @@ TABLE_CATEGORY get_table_category(const 
>      if ((name->length == SLOW_LOG_NAME.length) &&
>          (my_strcasecmp(system_charset_info,
>                        SLOW_LOG_NAME.str,
> +                      name->str) == 0))
> +    {
> +      return TABLE_CATEGORY_PERFORMANCE;
> +    }
> +
> +    if ((name->length == BACKUP_HISTORY_LOG_NAME.length) &&
> +        (my_strcasecmp(system_charset_info,
> +                      BACKUP_HISTORY_LOG_NAME.str,
> +                      name->str) == 0))
> +    {
> +      return TABLE_CATEGORY_PERFORMANCE;
> +    }
> +
> +    if ((name->length == BACKUP_PROGRESS_LOG_NAME.length) &&
> +        (my_strcasecmp(system_charset_info,
> +                      BACKUP_PROGRESS_LOG_NAME.str,
>                        name->str) == 0))
>      {
>        return TABLE_CATEGORY_PERFORMANCE;
>

Why do we need this change? Better if the patch changes as few as possible in 
the server code.

Rafal