Chuck,
Here are my answers.
Chuck Bell wrote:
> Rafal,
>
> I have to open and close the tables individually because I am using a write
> lock. The merlin folks want to be able to read from the tables at any time.
> Rather than debate this now, I suggest we defer this one until after the big
> push.
>
Ah, right. Then it is not as simple as I thought.
> I will look at the calls during Synch phase. I didn't think I had any but I
> am wrong it seems.
>
> Please look through below. I have questions for you.
>
> Chuck
>
>
>>> + /*
>>> + Record the backup id for this driver.
>>> + */
>>> + void set_backup_prog_id(ulonglong backup_prog_id)
>>> + { m_drv->backup_prog_id= backup_prog_id; };
>>> +
>>> + /*
>>> + Record the engine name this driver.
>>> + Note: must be called after set_backup_prog_id.
>>> + */
>>> + void set_backup_prog_engine_name(const char *engine_name)
>>> + { report_ob_engines(m_drv->backup_prog_id, engine_name); };
>> I'd not add progress reporting to the responsibilities of the
>> Pump class - it is
>> perfectly ok to report these things outside. Adding weakly
>> related tasks to a
>> class weakens modularity of the code...
>
> Ok, but I need a way to get the name of the drivers. This seemed the best
> place. Can you suggest another place?
The backup engine name is stored in Pump::m_name - you use it below.
>
>>> +
>>> private:
>>>
>>> static const uint m_buf_size= 2048; ///< Size of buffers
>> used for data transfers.
>>> @@ -513,6 +527,12 @@ int write_table_data(THD* thd, Backup_in
>>> continue;
>>>
>>> Scheduler::Pump *p= new Scheduler::Pump(no,*i,s);
>>> +
>>> + /*
>>> + Record the backup id and name for this driver.
>>> + */
>>> + p->set_backup_prog_id(info.backup_prog_id);
>>> + p->set_backup_prog_engine_name(p->m_name);
>> It seems pointless to pass objects member value to this
>> objects method - if
>> method needs this value it can read it directly.
>>
>> I opt for calling report_ob_engines() and such outside of the
>> Pump object.
>
> So where do I put it? I just need a way to get the name of the drivers when
> backup and restore begin. I'd gladly move it but I need you to tell me where
> the best location is.
>
I thought putting here:
report_ob_engines(info.backup_prog_id, p->m_name);
would do the trick - correct?
>>>
>>> if (!p || !p->is_valid())
>>> {
>>> @@ -625,6 +645,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);
>> PROBLEM:
>> I think it is not acceptable to have such complex thing
>> happen here between
>> lock()/unlock() calls. Complex because inside
>> report_ob_state() function we
>> spawn the locking thread, open progress table and potentially
>> write to disk.
>
> Ok, will move.
>
>>> /*
>>> Block commits.
>>>
>>> @@ -651,6 +672,8 @@ int write_table_data(THD* thd, Backup_in
>>> info.binlog_information.position= li.pos;
>>> memcpy(info.binlog_information.binlog_file_name,
>>> li.log_file_name, strlen(li.log_file_name));
>>> + if (strlen(li.log_file_name) > 0)
>>> +
>> info.binlog_information.binlog_file_name[strlen(li.log_file_name)]= 0;
>>> DBUG_PRINT("SYNC PHASE - binlog position : ", ("%d",
>> li.pos));
>>> DBUG_PRINT("SYNC PHASE - binlog filename : ", ("%s",
>> li.log_file_name));
>>> }
>>> @@ -674,6 +697,15 @@ int write_table_data(THD* thd, Backup_in
>>> if (error)
>>> goto error;
>>>
>>> + BACKUP_BREAKPOINT("bp_validity_point");
>>> + report_ob_vp_time(info.backup_prog_id, (my_time_t)skr);
>>> + report_ob_state(info.backup_prog_id, BUP_RUNNING);
>>> + BACKUP_BREAKPOINT("bp_running");
>>> +
>> I think this is executed after VP has been created (after
>> unlock() calls) which
>> is correct. But then the "bp_validity_point" breakpoint seems
>> missplaced - it is
>> already past the validity point.
>
> Hmmm...
>
You agree or disagree?
>>> + if (mysql_bin_log.is_open())
>>> + report_ob_binlog_info(info.backup_prog_id,
>> (int)info.binlog_information.position,
>>> +
>> info.binlog_information.binlog_file_name);
>>> +
>> Minor thing: instead of examining mysql_bin_log again (it was
>> already done above
>> when binlog pos was saved in the info structure) examine the
>> info structure to
>> know if there is anything to report.
>
> ??
Let's leave it - not important really.
>
>>> // get final data from drivers
>>> DBUG_PRINT("backup/data",("-- FINISH PHASE --"));
>
