Hi,
Patch is approved. One small suggestion included below.
Chuck
> ChangeSet@stripped, 2008-05-27 21:49:04+02:00,
> istruewing@stripped +4 -0
> Bug#36979 - merge-sync.test fails in pushbuild
>
> The test case failed randomly. When concurrent inserts are enabled,
> a SELECT in one connection does not always see rows inserted
> immediately before in another connection.
> See Bug#36618 - myisam insert not immediately visible to select
> from another client.
>
> Fixed by disabling concurrent inserts for this test case.
>
> mysql-test/r/merge-sync.result@stripped, 2008-05-27
> 21:49:02+02:00, istruewing@stripped +3 -1
> Bug#36979 - merge-sync.test fails in pushbuild
> Fixed result.
>
> mysql-test/t/disabled.def@stripped, 2008-05-27 21:49:02+02:00,
> istruewing@stripped +0 -1
> Bug#36979 - merge-sync.test fails in pushbuild
> Re-enabled the test case.
>
> mysql-test/t/merge-sync.test@stripped, 2008-05-27
> 21:49:02+02:00, istruewing@stripped +30 -1
> Bug#36979 - merge-sync.test fails in pushbuild
> Fixed test case.
>
> sql/sql_parse.cc@stripped, 2008-05-27 21:49:02+02:00,
> istruewing@stripped +12 -0
> Bug#36979 - merge-sync.test fails in pushbuild
> Added debug synchronization points.
>
> diff -Nrup a/mysql-test/r/merge-sync.result
> b/mysql-test/r/merge-sync.result
> --- a/mysql-test/r/merge-sync.result 2008-04-29 11:22:02 +02:00
> +++ b/mysql-test/r/merge-sync.result 2008-05-27 21:49:02 +02:00
> @@ -1,6 +1,7 @@
> SET DEBUG_SYNC= 'RESET';
> drop table if exists t1,t2,t3,t4,t5,t6;
> drop database if exists mysqltest;
> +SET GLOBAL concurrent_insert = 0;
> CREATE TABLE t1 (c1 INT) ENGINE= MyISAM;
> CREATE TABLE t2 (c1 INT) ENGINE= MRG_MYISAM UNION= (t1)
> INSERT_METHOD= LAST;
> connection con1
> @@ -147,5 +148,6 @@ SET DEBUG_SYNC= 'now WAIT_FOR locked';
> UNLOCK TABLES;
> connection con1
> connection default;
> -SET DEBUG_SYNC= 'RESET';
> DROP TABLE t1;
> +SET DEBUG_SYNC= 'RESET';
> +SET GLOBAL concurrent_insert = DEFAULT;
> diff -Nrup a/mysql-test/t/disabled.def b/mysql-test/t/disabled.def
> --- a/mysql-test/t/disabled.def 2008-05-26 17:29:07 +02:00
> +++ b/mysql-test/t/disabled.def 2008-05-27 21:49:02 +02:00
> @@ -54,6 +54,5 @@ character_set_database_func : Bug#36974
> ddl_i18n_utf8 : Bug#36975 ddl_i18n_utf8.test
> fail in pushbuild
> delayed_queue_size_basic_64 : Bug#36976
> delayed_queue_size_basic_64.test fails in pushbuild
> sql_low_priority_updates_func : Bug#36977
> sql_low_priority_updates_func.test fails in pushbuild
> -merge-sync : Bug#36979 merge-sync.test
> fails in pushbuild
> max_write_lock_count_basic_32 : Bug#36980
> max_write_lock_count_basic_32.test fails in pushbuild
>
> diff -Nrup a/mysql-test/t/merge-sync.test
> b/mysql-test/t/merge-sync.test
> --- a/mysql-test/t/merge-sync.test 2008-04-29 11:22:02 +02:00
> +++ b/mysql-test/t/merge-sync.test 2008-05-27 21:49:02 +02:00
> @@ -12,6 +12,16 @@
> SET DEBUG_SYNC= 'RESET';
> drop table if exists t1,t2,t3,t4,t5,t6;
> drop database if exists mysqltest;
> +#
> +# Due to a performance gaining "misplacement" of sending ok to the
> +# client, the server may not always see rows that have just been
> +# inserted by another connection. Fixed by disabling
> concurrent inserts.
> +# See Bug#36618 - myisam insert not immediately visible to select
> +# from another client.
> +# If you want to test it, remove the comment signs from the lines,
> +# that do also mention Bug#36618, and the two other there
> mentioned lines.
> +# Then change the set value in the next line.
> +SET GLOBAL concurrent_insert = 0;
> --enable_warnings
>
> #
> @@ -274,6 +284,16 @@ CREATE TABLE m1 (c1 INT) ENGINE=MRG_MYIS
> SIGNAL attach WAIT_FOR store_lock1';
> SET DEBUG_SYNC= 'before_myisammrg_store_lock
> SIGNAL store_lock2 WAIT_FOR flushed';
> + # Exploit Bug#36618 - myisam insert not immediately
> visible to select
> + # from another client
> + # Signal 'select_locking' to avoid waiting in the SET
> statement itself.
> + # Use EXECUTE 2 to survive execution in the SET statement itself.
> + # If you enable the 3 lines below, enable also the 2 lines, that
> + # SIGNAL select_locking, and don't run with an embedded server.
Should you add the include for not embedded and comment it out just in case?
> + #SET DEBUG_SYNC= 'now SIGNAL select_locking';
> + #SET DEBUG_SYNC= 'after_dispatch_net_end_statement
> + # WAIT_FOR select_locking EXECUTE 2';
> + #
> send INSERT INTO m1 VALUES (2);
> --echo connection default;
> connection default;
> @@ -295,7 +315,12 @@ FLUSH TABLE m1;
> disconnect con1;
> --echo connection default;
> connection default;
> +# Let con1 finish the INSERT statement when SELECT is in
> wait_for_lock().
> +#SET DEBUG_SYNC= 'wait_for_lock SIGNAL select_locking';
> SELECT * FROM m1;
> +# When concurrent_insert is not disabled, SELECT does not
> need to wait
> +# for lock. Kick INSERT out of its wait now.
> +#SET DEBUG_SYNC= 'now SIGNAL select_locking';
> # Clear debug_sync signal.
> SET DEBUG_SYNC= 'RESET';
> DROP TABLE m1, t1;
> @@ -378,7 +403,11 @@ UNLOCK TABLES;
> --echo connection default;
> connection default;
> #
> +DROP TABLE t1;
> +#
> # Clear debug_sync signal.
> SET DEBUG_SYNC= 'RESET';
> -DROP TABLE t1;
> +#
> +# Reset concurrent inserts. It was changed at top of this file.
> +SET GLOBAL concurrent_insert = DEFAULT;
>
> diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
> --- a/sql/sql_parse.cc 2008-05-22 20:29:39 +02:00
> +++ b/sql/sql_parse.cc 2008-05-27 21:49:02 +02:00
> @@ -1063,6 +1063,12 @@ bool dispatch_command(enum enum_server_c
> char *beginning_of_next_stmt= (char*) end_of_stmt;
>
> net_end_statement(thd);
> + /*
> + Allow to exploit Bug#36618 - myisam insert not immediately
> + visible to select from
> another client.
> + */
> + DEBUG_SYNC(thd, "after_dispatch_net_end_statement");
> +
> query_cache_end_of_result(thd);
> /*
> Multiple queries exits, execute them individually
> @@ -1427,6 +1433,12 @@ bool dispatch_command(enum enum_server_c
> }
>
> net_end_statement(thd);
> + /*
> + Allow to exploit Bug#36618 - myisam insert not immediately
> + visible to select from another client.
> + */
> + DEBUG_SYNC(thd, "after_dispatch_net_end_statement");
> +
> query_cache_end_of_result(thd);
>
> thd->proc_info= "closing tables";
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>
