This patch is good to push. I'll check the approved box when we finally decide
which patch to use as a solution for the bug.
Rafal
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 11:16:44-04:00, cbell@mysql_cab_desk. +3 -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 functionality removed pending more work in WL#4296
>
> mysql-test/t/disabled.def@stripped, 2008-03-19 11:16:35-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 11:16:35-04:00,
> cbell@mysql_cab_desk. +30 -327
> BUG##34858 Server crashes by backup tests
>
> Removed functionality pending WL#4296. This is to fix problems
> found from latest changes to server code.
>
> sql/backup/be_snapshot.cc@stripped, 2008-03-19 11:16:36-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.
>
> 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 11:16:35 -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 11:16:35 -04:00
> @@ -391,112 +391,11 @@ ulonglong report_ob_init(int process_id,
> const char *backup_file,
> const char *command)
> {
> - 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
> -
> DBUG_ENTER("report_ob_init()");
> -
> - 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();
> -
> - 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_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();
> -
> - if (host)
> - {
> - 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 (user)
> - {
> - if (table->field[ET_FIELD_USERNAME]->store(user,
> - strlen(user), system_charset_info))
> - goto end;
> - table->field[ET_FIELD_USERNAME]->set_notnull();
> - }
> -
> - if (user_comment)
> - {
> - if (table->field[ET_FIELD_COMMENT]->store(user_comment,
> - strlen(user_comment), system_charset_info))
> - goto end;
> - table->field[ET_FIELD_COMMENT]->set_notnull();
> - }
> -
> - if (backup_file)
> - {
> - 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();
> - }
> -
> - if (command)
> - {
> - if (table->field[ET_FIELD_COMMAND]->store(command,
> - strlen(command), system_charset_info))
> - goto end;
> - table->field[ET_FIELD_COMMAND]->set_notnull();
> - }
> - table->in_use= t;
> -
> - table->next_number_field=table->found_next_number_field;
> -
> - /*
> - Write the row.
> - */
> - if ((ret= table->file->ha_write_row(table->record[0])))
> - table->file->print_error(ret, MYF(0));
> -
> - /*
> - Get last insert id for row.
> - */
> - backup_id= table->file->insert_id_for_cur_row;
> - table->file->ha_release_auto_increment();
> -
> -end:
> -
> - ret= close_backup_progress_table(locking_thd);
> /*
> - Record progress update.
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> */
> - String str;
> - get_state_string(state, &str);
> - report_ob_progress(backup_id, "backup kernel", 0,
> - 0, 0, 0, 0, str.c_ptr());
> - DBUG_RETURN(backup_id);
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -516,59 +415,11 @@ int report_ob_binlog_info(ulonglong back
> 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.
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> */
> - 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);
> - DBUG_RETURN(ret);
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -586,12 +437,11 @@ end:
> int report_ob_error(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);
> - DBUG_RETURN(ret);
> + /*
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> + */
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -609,12 +459,11 @@ int report_ob_error(ulonglong backup_id,
> int report_ob_num_objects(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);
> - DBUG_RETURN(ret);
> + /*
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> + */
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -632,12 +481,11 @@ int report_ob_num_objects(ulonglong back
> int report_ob_size(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);
> - DBUG_RETURN(ret);
> + /*
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> + */
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -657,16 +505,11 @@ int report_ob_time(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);
> - if (stop)
> - update_online_backup_datetime_field(backup_id, "online_backup",
> - ET_FIELD_STOP_TIME, stop);
> - DBUG_RETURN(ret);
> + /*
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> + */
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -684,13 +527,11 @@ int report_ob_time(ulonglong backup_id,
> int report_ob_vp_time(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);
> - DBUG_RETURN(ret);
> + /*
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> + */
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -709,62 +550,11 @@ int report_ob_vp_time(ulonglong backup_i
> int report_ob_engines(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.
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> */
> - 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));
> -
> -end:
> -
> - ret= close_backup_progress_table(locking_thd);
> - DBUG_RETURN(ret);
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -782,20 +572,11 @@ end:
> int report_ob_state(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);
> /*
> - Record progress update.
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> */
> - get_state_string(state, &str);
> - report_ob_progress(backup_id, "backup kernel", 0,
> - 0, 0, 0, 0, str.c_ptr());
> -
> - DBUG_RETURN(ret);
> + DBUG_RETURN(0);
> }
>
> /**
> @@ -826,89 +607,11 @@ int report_ob_progress(ulonglong backup_
> int error_num,
> 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.
> + Functionality removed per BUG#34858 pending completion of WL#4296.
> */
> - 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);
> - }
> -
> - 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);
> + DBUG_RETURN(0);
> }
>
> /**
> 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 11:16:36 -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);
> }
>
>
