List:Commits« Previous MessageNext Message »
From:anders Date:January 10 2011 7:56am
Subject:Re: bzr commit into mysql-trunk branch (daogang.qu:3454) Bug#58784
View as plain text  
Hi Daogang,
Nice work.

Please find my review comments below.

STATUS
------
 
  Not Approved.

REQUIRED CHANGES
----------------
RC1. I am ok with the patch, but I think there is a better way
to handle the debug.
It is similar with your patch, except that write_delayed() will
wait an signal, but not sleep.

the code like(I didn't verify the code):
in write_delayed()
+  DBUG_EXECUTE_IF("dbug.waiting_for_delayed_insert_queue_is_empty",
+                  {
+                    if (di->insert_delayed > 0) 
+                    {
+                      const char act[]=
+                        "now "
+                        "wait_for signal.has_inserted";
+                      DBUG_ASSERT(opt_debug_sync_timeout > 0);
+                      DBUG_ASSERT(!debug_sync_set_action(current_thd,
+                                                STRING_WITH_LEN(act)));
+                    }
+                  };);

 
in handle_inserts()

+ DEBUG_SYNC("handle_inserts");
+ Please check the sql/debug_sync.cc.

In the way, the extra variable is not needed.

REQUESTS
--------
NO

SUGGESTIONS
-----------
NO

DETAILS 
-------
NO

On Thu, 2011-01-06 at 03:42 +0000, daogang.qu@stripped wrote:
> #At file:///home/daogang/bzrwork/bug58784/mysql-trunk/ based on
> revid:marc.alff@stripped
> 
>  3454 daogang.qu@stripped	2011-01-06
>       Bug #58784  	rpl_row_ignorable_event fails on PB2
>       
>       In RBR, The rows are inserted to a queue by the thread executing
>       the 'INSERT DELAYED' statement, and are taken out from the queue
>       by the handler thread to do the real insertion. Because these two
>       threads are running in parallel, there is a possibility that they
>       are run in an interleaved manner, and result in different number
>       of table_map and rows events.
>             
>       Added a variable and a debug option for the test to make the
>       binlog of multi 'INSERT DELAYED ...' stmt stable by forcing
>       every value is executed into one execution series, and then
>       each value will be binlogged into a separate rows event with
>       its table map event.
>      @ sql/sql_insert.cc
>         Added a 'queue_is_empty' variable to indicate that the delayed
>         insert queue is empty until the last row from the queue is
>         executed completely.
> 
>     modified:
>       sql/sql_insert.cc
> === modified file 'sql/sql_insert.cc'
> --- a/sql/sql_insert.cc	2010-12-22 13:23:59 +0000
> +++ b/sql/sql_insert.cc	2011-01-06 03:42:49 +0000
> @@ -1871,6 +1871,7 @@ public:
>    mysql_cond_t cond, cond_client;
>    volatile uint tables_in_use,stacked_inserts;
>    volatile bool status;
> +  bool queue_is_empty;
>    /**
>      When the handler thread starts, it clones a metadata lock ticket
>      which protects against GRL and ticket for the table to be inserted.
> @@ -1895,7 +1896,8 @@ public:
>  
>    Delayed_insert()
>      :locks_in_memory(0), table(0),tables_in_use(0),stacked_inserts(0),
> -     status(0), handler_thread_initialized(FALSE), group_count(0)
> +     status(0), queue_is_empty(TRUE), handler_thread_initialized(FALSE),
> +     group_count(0)
>    {
>      DBUG_ENTER("Delayed_insert constructor");
>      thd.security_ctx->user=(char*) delayed_user;
> @@ -2346,7 +2348,7 @@ int write_delayed(THD *thd, TABLE *table
>  
>    thd_proc_info(thd, "waiting for handler insert");
>    DBUG_EXECUTE_IF("waiting_for_delayed_insert_queue_is_empty",
> -                  while(di->stacked_inserts) sleep(1););
> +                  while(!di->queue_is_empty) sleep(1););
>    mysql_mutex_lock(&di->mutex);
>    while (di->stacked_inserts >= delayed_queue_size &&
> !thd->killed)
>      mysql_cond_wait(&di->cond_client, &di->mutex);
> @@ -2431,6 +2433,7 @@ int write_delayed(THD *thd, TABLE *table
>  
>    di->rows.push_back(row);
>    di->stacked_inserts++;
> +  di->queue_is_empty= FALSE;
>    di->status=1;
>    if (table->s->blob_fields)
>      unlink_blobs(table);
> @@ -3104,6 +3107,8 @@ bool Delayed_insert::handle_inserts(void
>    }
>    query_cache_invalidate3(&thd, table, 1);
>    mysql_mutex_lock(&mutex);
> +  DBUG_EXECUTE_IF("waiting_for_delayed_insert_queue_is_empty",
> +                  queue_is_empty= TRUE;);
>    DBUG_RETURN(0);
>  
>   err:
> 
> text/bzr-bundle type attachment
> (bzr/daogang.qu@stripped)
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: daogang.qu@stripped\
> #   tj7y1rhbz0oiludy
> # target_branch: file:///home/daogang/bzrwork/bug58784/mysql-trunk/
> # testament_sha1: 5607620fb40e1d232e0f42821f94ed8c8cc81f5a
> # timestamp: 2011-01-06 11:42:54 +0800
> # base_revision_id: marc.alff@stripped\
> #   yab7b8mxeng998ia
> # 
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWe0OaWYAAiHfgHAxeOf//3+l
> XmC////wUAYN03g3bHdcLVgOgbY4aISYmp+ianinppkNGmppmSaNGgwjTQ0HqDjJk00wmRkDAjE0
> YIwg0aYABBoTEJkyJNpTTCejSAAAZBoBpoAaCJkTKPCj1PQanqNPUyABoAGgGTagSRCZCZGTQFPT
> EU8KbTU9IDNNTTQANPU8WQIeifA+ZI9rBtkljtyIlSYD9rmmhcfCAC4r5Ym3oFtITyfF/k/5emm+
> RDAyJwAv0y6bebl+dzMwWuRDrWIMXPWwTilRoPZM3KqVz+Q5pAdl8YKgjaWj8OAN+2xGFqRXwuEz
> EX0N+PqNf42MYtxUezOIEFROrilLGKS2m+czOVawq4aAgYeO5vIMbREweRulOsZXW15SNlCQNhzZ
> WVjTfgx4sDmI5gZgmpqMLCSXsdvYYWWV5E7AmoZJ8PBczrAnurmIfcbuqvXyx1dSmKtl3K+zebdd
> k9CvRekNjxSpZSprvXezVIMzbOI9/4KrhNeFATIGR1Cg6RFSCzvUIVMSWmb9nz9BXoiwzW5+dFJA
> 3kyqZXsLE4cdNJIKpeE41tqu2WGv4mfSpQ7badO3lkHFq/Jalmt1sv94iz1lBGBXwibmDoFv8YkL
> t+4rxqqy1qQXW1G7oVHLDWI4SuEcVyCfs3BNXfB3NmvRk5N3c2OszyiU6jNCQnAlREDto3pOsGEO
> UFGwYphLJQU8Oo5lECxHYUtzVfrj8Z5KHr61gq9kxhZqcNN09U1tsnte8zZpKvEjljnTltnExo9S
> XNkGYlXhshashQshDVygr5rZilXI+C7SqJlm1pBUO64jEplMuZWwQEZaprQaQhkLFJSrCpbxW2Hc
> a6YSQYpTBYBFtU47IkwUSlAnSBoFVgIyPHn0GDo1hQqhqkuGE5t8/SKCJqMuwfAeecM+RhA7e6Cl
> lBIWjSZv/dQeGi701d0UTfMa8g0h4dyGwRCgXCgRyOChr9bcFiN2z0Io9R36UcadRA0gEpnWwkNv
> gmWANo0jfWSflWNW+bRAHRFfIvIh9Oj2V07vNWFPzNU8osj0yNiWbWkxhiOp9bEEZjbKsa3VdOOK
> UAi8zo9/x8/doTHQvgQwupp2ZSqE27O8WyE4GAZLPdMtkeRddLeEjQP+NqX3yw4Vyg7BNIxx8c6u
> FWzNNgdM7eWnp877iiltFElRW7cnkPMG/ZGMpZxFCDidXYpNAQgG0xc4SkBzzoOms1WJUrYZhFMC
> 1hImetWpAgLERHJlqz2syfBTOsGOsR9UDKgbpXCJsBGSIoTIBODKM1QGhQXdcRWb9p6uqtYcOA5d
> uoWLefQZ6EnEotykVUdJZCKm+daIZgGV7etmnzm/RG/lQiwFzZu0qvKmvr5pGOlc5BJ4KapTtOKy
> yMDgcHVkh14d16xW/X7JYtZ0eofWyiw3dE2tk8IJZ3cIpYi63z1eiXz7+lcOwb9n6QwInMTAebGl
> g+1kbEm7BjjM8FB7JPru3A0KpKL6etg6rt20VRasQt5A+F+U5tfmKnFNHv5apTNLMqgml2hyWObG
> vt+ixiVou5QWB/ClXRu68Jw2dXR7UwXhU6+cUfTqia9xWwxgDOw3C23kWXhNjan2QhkRebOKJdgC
> 720Gk8yuU5SUTGm3DtxcQnIAcFNxUKLJHpQJjgyqQM5KKAMzEDCCIJST9PJSh31uTf1+1tEkFgiT
> A7sOmYMzrHqxFvhqrnu3XpNzFlUcUKdyTdXKV6TRuNXeEggHJ01ttOaVborzmWNQapkeDsEiwDps
> hmqpCc3At2K0Cm5KsOfyfxZvTRhc4mZkD62DQlB4su8KcUoxRDHoc7BRJXtyaxZfZv9RPVfmgy9L
> BFkKM8GORuudxumeU3ik28hFq8mwQP1tfOsHrEZveRdPecMWZFUmRiWlITPqN4CnmJt6adMdGOAI
> wBhA4GnqC0Iy0Jwsw9V3pzKk2gXGYItiN7m1RpPMYr6nlqFhfRk1iUAcI0epk8qnoBTsS3Xi0rSD
> jJRSPd1dyzLnht0YLLFpMlto9Yc/GYSzqxMKtUOPZMtsUlNFy2p061D4aKOM8XZk+TQ+Aoqi0Ea6
> AqiUonLxUkvUX+LuSKcKEh2hzSzA
> 
> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1

-- 
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer

Email : Anders.Song@stripped
Skype : libing.song
MSN   : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================

Thread
bzr commit into mysql-trunk branch (daogang.qu:3454) Bug#58784daogang.qu6 Jan
  • Re: bzr commit into mysql-trunk branch (daogang.qu:3454) Bug#58784anders10 Jan
    • Re: bzr commit into mysql-trunk branch (daogang.qu:3454) Bug#58784Daogang Qu11 Jan