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.
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?
>
> > +
> > 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.
> >
> > 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...
>
> > + 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.
??
>
> > // get final data from drivers
> > DBUG_PRINT("backup/data",("-- FINISH PHASE --"));
