Chuck,
Yes, if you agree with the changes. I'll tick the review box then.
Rafal
Chuck Bell wrote:
> Rafal,
>
> Is it ok to push once I make the documentation changes?
>
> Chuck
>
> -----Original Message-----
> From: Rafal Somla [mailto:rsomla@stripped]
> Sent: Tuesday, October 30, 2007 3:50 AM
> To: cbell@stripped
> Cc: commits@stripped
> Subject: Re: bk commit into 5.2 tree (cbell:1.2606)
>
> Hi Chuck,
>
> Excellent documentation on breakpoints! There is one point - timeouts -
> which
> needs clarifying. See my comments below.
>
> cbell@stripped wrote:
>> Below is the list of changes that have just been committed into a local
>> 5.2 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, 2007-10-24 17:29:11-04:00, cbell@mysql_cab_desk. +6 -0
>> WL#3324 : Online Backup: Basic testing
>>
>> This patch adds documentation for using breakpoints in the backup
> system. It also changes the
>> name of the macro to correspond with the breakpoint concept.
>>
>> CMakeLists.txt@stripped, 2007-10-24 17:28:59-04:00, cbell@mysql_cab_desk. +4
> -0
>> WL#3324 : Online Backup: Basic testing
>>
>> Added script item to allow using EXTRA_DEBUG on Windows.
>>
>> sql/backup/data_backup.cc@stripped, 2007-10-24 17:29:01-04:00,
> cbell@mysql_cab_desk. +5 -5
>> WL#3324 : Online Backup: Basic testing
>>
>> This patch renames the BACKUP_SYNC macro to BACKUP_BREAKPOINT to
> correspond with documentation
>> and behavior.
>>
>> sql/backup/debug.h@stripped, 2007-10-24 17:29:01-04:00, cbell@mysql_cab_desk.
> +203 -21
>> WL#3324 : Online Backup: Basic testing
>>
>> This patch renames the BACKUP_SYNC macro to BACKUP_BREAKPOINT to
> correspond with documentation
>> and behavior.
>>
>> sql/backup/sql_backup.cc@stripped, 2007-10-24 17:29:02-04:00,
> cbell@mysql_cab_desk. +4 -4
>> WL#3324 : Online Backup: Basic testing
>>
>> This patch renames the BACKUP_SYNC macro to BACKUP_BREAKPOINT to
> correspond with documentation
>> and behavior. It also includes documentation on how to use the
> breakpoints for debugging and
>> testing.
>>
>> sql/item_func.cc@stripped, 2007-10-24 17:29:00-04:00, cbell@mysql_cab_desk.
> +13 -2
>> WL#3324 : Online Backup: Basic testing
>>
>> Added debug_sync_point + lock name to the state of the process list.
>>
>> win/configure.js@stripped, 2007-10-24 17:29:03-04:00, cbell@mysql_cab_desk.
> +1 -0
>> WL#3324 : Online Backup: Basic testing
>>
>> Added script item to allow using EXTRA_DEBUG on Windows.
>>
>> diff -Nrup a/CMakeLists.txt b/CMakeLists.txt
>> --- a/CMakeLists.txt 2007-08-06 17:14:46 -04:00
>> +++ b/CMakeLists.txt 2007-10-24 17:28:59 -04:00
>> @@ -96,6 +96,10 @@ IF(CYBOZU)
>> ADD_DEFINITIONS(-DCYBOZU)
>> ENDIF(CYBOZU)
>>
>> +IF(EXTRA_DEBUG)
>> + ADD_DEFINITIONS(-D EXTRA_DEBUG)
>> +ENDIF(EXTRA_DEBUG)
>> +
>> # in some places we use DBUG_OFF
>> SET(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -DDBUG_OFF")
>> SET(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}
> -DDBUG_OFF")
>> diff -Nrup a/sql/backup/data_backup.cc b/sql/backup/data_backup.cc
>> --- a/sql/backup/data_backup.cc 2007-09-11 05:29:29 -04:00
>> +++ b/sql/backup/data_backup.cc 2007-10-24 17:29:01 -04:00
>> @@ -440,7 +440,7 @@ int write_table_data(THD*, Backup_info &
>> inactive.elements));
>>
>> DBUG_PRINT("backup/data",("-- INIT PHASE --"));
>> - BACKUP_SYNC("data_init");
>> + BACKUP_BREAKPOINT("data_init");
>>
>> /*
>> Poll "at end" drivers activating inactive ones on the way.
>> @@ -503,7 +503,7 @@ int write_table_data(THD*, Backup_info &
>>
>> // prepare for VP
>> DBUG_PRINT("backup/data",("-- PREPARE PHASE --"));
>> - BACKUP_SYNC("data_prepare");
>> + BACKUP_BREAKPOINT("data_prepare");
>>
>> if (sch.prepare())
>> goto error;
>> @@ -530,17 +530,17 @@ int write_table_data(THD*, Backup_info &
>>
>> // VP creation
>> DBUG_PRINT("backup/data",("-- SYNC PHASE --"));
>> - BACKUP_SYNC("data_lock");
>> + BACKUP_BREAKPOINT("data_lock");
>> if (sch.lock())
>> goto error;
>>
>> - BACKUP_SYNC("data_unlock");
>> + BACKUP_BREAKPOINT("data_unlock");
>> if (sch.unlock())
>> goto error;
>>
>> // get final data from drivers
>> DBUG_PRINT("backup/data",("-- FINISH PHASE --"));
>> - BACKUP_SYNC("data_finish");
>> + BACKUP_BREAKPOINT("data_finish");
>>
>> while (sch.finish_count > 0)
>> if (sch.step())
>> diff -Nrup a/sql/backup/debug.h b/sql/backup/debug.h
>> --- a/sql/backup/debug.h 2007-06-27 11:37:32 -04:00
>> +++ b/sql/backup/debug.h 2007-10-24 17:29:01 -04:00
>> @@ -1,7 +1,7 @@
>> #ifndef _BACKUP_DEBUG_H
>> #define _BACKUP_DEBUG_H
>>
>> -#define BACKUP_SYNC_TIMEOUT 300
>> +#define BACKUP_BREAKPOINT_TIMEOUT 300
>>
>> /*
>> TODO
>> @@ -45,34 +45,216 @@ namespace backup {
>> DBUG_EXECUTE_IF("backup_error_test", backup::test_error_flag= (X);); \
>> } while(0)
>>
>> -/*
>> - Macros for creating synchronization points in tests.
>> -
>> - Usage
>> -
>> - In the backup code:
>> - BACKUP_SYNC("<synchronization point name>");
>> -
>> - In a client:
>> - SELECT get_lock("<synchronization point name>",<timeout>);
>> - ...
>> - SELECT release_lock("<synchronization point name>");
>> +/**
>> + @page BACKUP_BREAKPOINT Online Backup Breakpoints
>> + Macros for creating breakpoints during testing.
>> +
>> + @section WHAT What are breakpoints?
>> + Breakpoints are devices used to pause the execution of the backup
> system
>> + at a certain point in the code. There is a timeout that you can specify
>> + when you set the breakpoint which will enable execution to continue
> after
>> + the period in seconds expires.
>
> I think this is a bit confusing. Currently, one can specify timeout only in
> get_lock() SQL function, not in the code. The BACKUP_BREAKPOINT() has fixed
> timeout given by BACKUP_BREAKPOINT_TIMEOUT macro.
>
> So, we have two different timeouts with different effects
>
> - when get_lock() timeout expires, the client gives up and doesn't activate
> the
> breakpoint.
>
> - if code stops at activated breakpoint and BACKUP_BREAKPOINT_TIMEOUT
> expires,
> the code will continue to execute.
>
> Only the first timeout is configurable by user (client).
>
>> +
>> + The best use of these breakpoints is for pausing execution at critical
>> + points in the backup code to allow proper testing of certain features.
>> + For example, suppose you wanted to ensure the Consistent Snapshot
> driver
>> + was working properly. To do so, you would need to ensure no new @INSERT
>> + statements are executed while the data is being backed up. If you use
>> + a breakpoint, you can set the breakpoint to pause the backup kernel at
>> + the point where it has set the consistent read and is reading rows.
>> + You can then insert some rows and release the breakpoint. The result
>> + should contain all of the rows in the table except those that were
>> + inserted once the consistent read was set.
>> +
>> + @section USAGE How to use breakpoints.
>> + To make a breakpoint available, you must add a macro call to the code.
>> + Simply insert the macro call as follows. The @c breakpoint_name is a
>> + text string that must be unique among the breakpoints. It is used in
>> + the macro as a means of tagging the code for pausing and resuming
>> + execution. Once the code is compiled, you can use a client connection
>> + to set and release the breakpoint.
>> +
>> +
> <b><c>BACKUP_BREAKPOINT("<breakpoint_name>");</c></b>
>> +
>> + Breakpoints use the user-defined locking functions @c get_lock() to set
>> + the breakpoint and @c release_lock() to release it.
>> +
>> + @subsection SET Setting breakpoints.
>> + To set an existing breakpoint, issue the following command where @c
>> + timeout is the number of seconds execution will wait for the release
>> + before continuing execution.
>
> Hmm, I think it is confused again. The timeout specifies how long we wait
> for
> the lock before giving up (see docs of get_lock() function).
>
>> +
>> + <b><c>SELECT
> get_lock("<breakpoint_name>",<timeout>);</c></b>
>> +
>> + @subsection RELEASE Releasing breakpoints.
>> + To release an existing breakpoint, issue the following command. This
>> + releases execution allow the system to continue.
>> +
>> + <b><c>SELECT
> release_lock("<breakpoint_name>");</c></b>
>> +
>> + @subsection EXAMPLE Example - Testing the Consistent Snapshot Driver
>> + To test the consistent snapshot driver, we can make use of the @c
>> + backup_cs_locked breakpoint to pause execution after the consistent
> read
>> + is initiated and before all of the rows from the table have been read.
>> + Consider an InnoDB table with the following structure as our test
> table.
>> +
>> + <c>CREATE TABLE t1 (a INT) ENGINE=INNODB;</c>
>> +
>> + To perform this test using breakpoints, we need two client connections.
>> + One will be used to execute the backup command and another to set and
>> + release the breakpoint. In the first client, we set the breakpoint with
>> + the <c>SELECT get_lock("backup_cs_locked", 100);</c> command. In
> the
>> + second client, we start the execution of the backup. We can return to
>> + the first client and issue several @INSERT statements then issue the
>> + <c>SELECT release_lock("backup_cs_locked");</c> command to release
> the
>> + breakpoint.
>> +
>> + We can then return to the second client, select all of the rows from
> the
>> + table to verify the rows were inserted. We can verify that the
> consistent
>> + snapshot worked by restoring the database (which is a destructive
> restore)
>> + and then select all of the rows. This will show that the new rows
>> + inserted while the backup was running were not inserted into the table.
>> + The following shows the output of the commands as described.
>> +
>> + <b>First Client</b>
>> + @code mysql> SELECT * FROM t1;
>> + +---+
>> + | a |
>> + +---+
>> + | 1 |
>> + | 2 |
>> + | 3 |
>> + +---+
>> + 3 rows in set (0.00 sec)
>> +
>> + mysql> SELECT get_lock("backup_cs_locked", 100);
>> + +-----------------------------------+
>> + | get_lock("backup_cs_locked", 100) |
>> + +-----------------------------------+
>> + | 1 |
>> + +-----------------------------------+
>> + 1 row in set (0.00 sec) @endcode
>> +
>> + <b>Second Client</b>
>> + @code mysql> BACKUP DATABASE test TO 'test.bak'; @endcode
>> +
>> + Note: The backup will pause while the breakpoint is set (the lock is
> held).
>> +
>> + <b>First Client</b>
>> + @code mysql> INSERT INTO t1 VALUES (101), (102), (103);
>> + Query OK, 3 rows affected (0.02 sec)
>> + Records: 3 Duplicates: 0 Warnings: 0
>> +
>> + mysql> SELECT * FROM t1;
>> + +-----+
>> + | a |
>> + +-----+
>> + | 1 |
>> + | 2 |
>> + | 3 |
>> + | 101 |
>> + | 102 |
>> + | 103 |
>> + +-----+
>> + 6 rows in set (0.00 sec)
>> +
>> + mysql> SELECT release_lock("backup_cs_locked");
>> + +----------------------------------+
>> + | release_lock("backup_cs_locked") |
>> + +----------------------------------+
>> + | 1 |
>> + +----------------------------------+
>> + 1 row in set (0.01 sec) @endcode
>> +
>> + <b>Second Client</b>
>> + @code +------------------------------+
>> + | Backup Summary |
>> + +------------------------------+
>> + | header = 14 bytes |
>> + | meta-data = 120 bytes |
>> + | data = 30 bytes |
>> + | -------------- |
>> + | total 164 bytes |
>> + +------------------------------+
>> + 5 rows in set (33.45 sec)
>> +
>> + mysql> SELECT * FROM t1;
>> + +-----+
>> + | a |
>> + +-----+
>> + | 1 |
>> + | 2 |
>> + | 3 |
>> + | 101 |
>> + | 102 |
>> + | 103 |
>> + +-----+
>> + 6 rows in set (0.00 sec)
>> +
>> + mysql> RESTORE FROM 'test.bak';
>> + +------------------------------+
>> + | Restore Summary |
>> + +------------------------------+
>> + | header = 14 bytes |
>> + | meta-data = 120 bytes |
>> + | data = 30 bytes |
>> + | -------------- |
>> + | total 164 bytes |
>> + +------------------------------+
>> + 5 rows in set (0.08 sec)
>> +
>> + mysql> SELECT * FROM t1;
>> + +---+
>> + | a |
>> + +---+
>> + | 1 |
>> + | 2 |
>> + | 3 |
>> + +---+
>> + 3 rows in set (0.00 sec)@endcode
>> +
>> + Note: The backup will complete once breakpoint is released (the lock is
>> + released).
>> +
>> + @section BREAKPOINTS Breakpoints
>> + The following are the available breakpoints included in the code.
>> +
>> + - <b>backup_command</b> Occurs at the start of the backup
> operation.
>> + - <b>data_init</b> Occurs at the start of the <b>INITIALIZE
> PHASE</b>.
>> + - <b>data_prepare</b> Occurs at the start of the <b>PREPARE
> PHASE</b>.
>> + - <b>data_lock</b> Occurs at the start of the <b>SYNC
> PHASE</b>.
>> + - <b>data_unlock</b> Occurs after the unlock calls.
>
> I think this breakpoint is *before* the unlock calls.
>
>> + - <b>data_finish</b> Occurs at the start of the <b>FINISH
> PHASE</b>.
>> + - <b>backup_meta</b> Occurs before the call to
> write_meta_data().
>> + - <b>backup_data</b> Occurs before the call to
> write_table_data().
>> + - <b>backup_done</b> Occurs after the call to write_table_data()
> returns.
>
> Perhaps better to not use function names, but describe operations. E.g.,
> backup_meta is a breakpoint before header and meta data is written to the
> image.
>
>> + - <b>backup_cs_locked</b> Consistent Snapshot - after the
> consistent
>> + read has been initiated but before rows are
> read.
>> + - <b>backup_cs_open_tables</b> Consistent Snapshot - before the
> call
> to
>> + open and lock tables.
>> + - <b>backup_cs_reading</b> Consistent Snapshot - occurs during
> read.
>> +
>> + @section NOTES Developer Notes
>> + - Breakpoints can be used in debug builds only. You must compile
>> + the code using the @c DEBUG_EXTRA preprocessor directive.
>> + - When adding breakpoints, you must add a list item for each breakpoint
>> + to the documentation for breakpoints. See the code for the macro
>> + definition in @ref debug.h for details.
>>
>> - If the lock is kept by a client, server code will wait on the
> corresponding
>> - BACKUP_SYNC() until it is released.
>> -
>> - Consider: set thd->proc_info when waiting on lock
>> */
>>
>> -#define BACKUP_SYNC(S) \
>> +/*
>> + Consider: set thd->proc_info when waiting on lock
>> +*/
>> +#define BACKUP_BREAKPOINT(S) \
>> do { \
>> - DBUG_PRINT("backup",("== synchronization on '%s' ==",(S))); \
>> - DBUG_SYNC_POINT((S),BACKUP_SYNC_TIMEOUT); \
>> + DBUG_PRINT("backup",("== breakpoint on '%s' ==",(S))); \
>> + DBUG_SYNC_POINT((S),BACKUP_BREAKPOINT_TIMEOUT); \
>> } while (0)
>>
>> #else
>>
>> -#define BACKUP_SYNC(S)
>> +#define BACKUP_BREAKPOINT(S)
>> #define TEST_ERROR FALSE
>> #define TEST_ERROR_IF(X)
>>
>> diff -Nrup a/sql/backup/sql_backup.cc b/sql/backup/sql_backup.cc
>> --- a/sql/backup/sql_backup.cc 2007-10-03 05:10:44 -04:00
>> +++ b/sql/backup/sql_backup.cc 2007-10-24 17:29:02 -04:00
>> @@ -73,7 +73,7 @@ execute_backup_command(THD *thd, LEX *le
>> DBUG_ENTER("execute_backup_command");
>> DBUG_ASSERT(thd && lex);
>>
>> - BACKUP_SYNC("backup_command");
>> + BACKUP_BREAKPOINT("backup_command");
>>
>> backup::Location *loc= backup::Location::find(lex->backup_dir);
>>
>> @@ -285,7 +285,7 @@ int mysql_backup(THD *thd,
>>
>> size_t start_bytes= s.bytes;
>>
>> - BACKUP_SYNC("backup_meta");
>> + BACKUP_BREAKPOINT("backup_meta");
>>
>> DBUG_PRINT("backup",("Writing image header"));
>>
>> @@ -299,13 +299,13 @@ int mysql_backup(THD *thd,
>>
>> DBUG_PRINT("backup",("Writing table data"));
>>
>> - BACKUP_SYNC("backup_data");
>> + BACKUP_BREAKPOINT("backup_data");
>>
>> if (backup::write_table_data(thd,info,s))
>> goto error;
>>
>> DBUG_PRINT("backup",("Backup done."));
>> - BACKUP_SYNC("backup_done");
>> + BACKUP_BREAKPOINT("backup_done");
>>
>> info.total_size= s.bytes - start_bytes;
>>
>> diff -Nrup a/sql/item_func.cc b/sql/item_func.cc
>> --- a/sql/item_func.cc 2007-08-26 18:38:29 -04:00
>> +++ b/sql/item_func.cc 2007-10-24 17:29:00 -04:00
>> @@ -3402,8 +3402,19 @@ void debug_sync_point(const char* lock_n
>> /*
>> Structure is now initialized. Try to get the lock.
>> Set up control struct to allow others to abort locks
>> - */
>> - THD_SET_PROC_INFO(thd, "User lock");
>> +
>> + Note: The size for the proc_info string is set to the same
>> + size as the "state" member of the processlist_fields_info
>> + structure in sql_show.cc.
>> +
>> + See processlist_fields_info[6].field_length.
>> + */
>> + char proc_info[64];
>> + if ((sizeof("debug_sync_point: ") + sizeof(lock_name)) <= 64)
>> + my_sprintf(proc_info,(proc_info, "debug_sync_point: %s", lock_name));
>> + else
>> + my_sprintf(proc_info, (proc_info, "debug_sync_point"));
>> + THD_SET_PROC_INFO(thd, proc_info);
>> thd->mysys_var->current_mutex= &LOCK_user_locks;
>> thd->mysys_var->current_cond= &ull->cond;
>>
>> diff -Nrup a/win/configure.js b/win/configure.js
>> --- a/win/configure.js 2007-08-06 17:14:47 -04:00
>> +++ b/win/configure.js 2007-10-24 17:29:03 -04:00
>> @@ -46,6 +46,7 @@ try
>> case "WITH_PARTITION_STORAGE_ENGINE":
>> case "__NT__":
>> case "CYBOZU":
>> + case "EXTRA_DEBUG":
>> case "EMBED_MANIFESTS":
>> case "WITH_EMBEDDED_SERVER":
>> configfile.WriteLine("SET (" + args.Item(i) + "
> TRUE)");
>>
>
> Rafal
>
