Hi Ingo,
I have looked at your test conversion patch and I think it is fine. I agree with
your point of view that synchronization point names should refer to the place in
the code where the point is placed, not to its role in a test. Still some names
could be discussed, like "wait_lock_global_read_lock" which doesn't indicate
clearly where the wait happens - potentially we can wait for global read lock in
many places in the code. However, this is really a minor issue in my opinion so
I'm fine with the names as they are now.
I also buy your argument about removing sync points which are not used now.
Thanks for helping us with that!
Rafal
Ingo Struewing wrote:
> diff -Nrup a/sql/sql_insert.cc b/sql/sql_insert.cc
> --- a/sql/sql_insert.cc 2008-04-21 12:20:17 +02:00
> +++ b/sql/sql_insert.cc 2008-06-06 16:30:25 +02:00
> @@ -61,7 +61,6 @@
> #include "sql_show.h"
> #include "slave.h"
> #include "rpl_mi.h"
> -#include "backup/debug.h"
> #include "sql_audit.h"
>
> #ifndef EMBEDDED_LIBRARY
> @@ -620,8 +619,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
> /*
> Breakpoints for backup testing.
> */
I think above comment (and similar comments elsewhere) should be also removed -
these synchronization points can
be used for whatever purposes.
> - BACKUP_BREAKPOINT("backup_cs_reading");
> - BACKUP_BREAKPOINT("commit_blocker_step_1");
> + DEBUG_SYNC(thd, "after_insert_locked_tables");
> thd_proc_info(thd, "init");
> thd->used_tables=0;
> values= its++;
