List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:May 12 2010 6:12am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453)
Bug#52704
View as plain text  
Luís Soares wrote:
> Hi Zhenxing,
> 
>   Nice Work. 
>   Please find my review comments below.
> 
> STATUS
> ------
>  
>   Not approved. (missing some test cases and there is also a
>                  request for feedback).
> 
> REQUIRED CHANGES
> ----------------
>  
>   RC1. Please add a test case, covering several scenarios:
> 
>        1. regular insert/updates
>        2. insert delayed
>        3. inserts in events 

OK

>        4. slow query log tests (with TIMESTAMP changed)
> 

Slow query log is not affected by this patch, so I think this test can
be skipped.

> REQUESTS
> --------
>  
>   R1. This is a request for feedback. 
> 
>       What are the implications for slow query logging? Should
>       this new real_start_time be now used instead of start_time?
> 

slow query logging used start_utime (in microseconds), which is not
affected by TIMESTAMP, so no need to change that.

> SUGGESTIONS
> -----------
>  n/a
> 
> DETAILS 
> -------
>  n/a
> 
> 
> On Sat, 2010-05-01 at 12:03 +0000, He Zhenxing wrote:
> > #At file:///media/sda3/work/mysql/bzrwork/b52704/5.1-bugteam/ based on
> revid:svoj@stripped
> > 
> >  3453 He Zhenxing	2010-05-01
> >       Bug#52704 exec_time=4294967295
> >       
> >       thd->start_time was used to calculate the exec_time of the query,
> >       since thd->start_time can be affected by TIMESTAMP session variable,
> >       this would result in a very big value when TIMESTAMP is set to a
> >       future time before execution a query.
> >       
> >       Fixed by adding a new varialbe thd->real_start_time, which will
> >       not be affected by TIMESTAMP, and use this to calculate the
> >       execution time of the query.
> >      @ sql/log_event.cc
> >         use real_start_time to calculate the execution time of the query, which
> is not affected by the TIMESTAMP session variable
> >      @ sql/sql_class.h
> >         add real_start_time to record the real system start time of the query,
> which is not affected by the TIMESTAMP session variable
> > 
> >     modified:
> >       sql/log_event.cc
> >       sql/sql_class.h
> > === modified file 'sql/log_event.cc'
> > --- a/sql/log_event.cc	2010-03-28 11:57:33 +0000
> > +++ b/sql/log_event.cc	2010-05-01 12:03:34 +0000
> > @@ -2394,7 +2394,8 @@ Query_log_event::Query_log_event(THD* th
> >    error_code= errcode;
> >  
> >    time(&end_time);
> > -  exec_time = (ulong) (end_time  - thd_arg->start_time);
> > +  DBUG_ASSERT(end_time >= thd_arg->real_start_time);
> > +  exec_time = (ulong) (end_time  - thd_arg->real_start_time);
> >    /**
> >      @todo this means that if we have no catalog, then it is replicated
> >      as an existing catalog of length zero. is that safe? /sven
> > @@ -4241,7 +4242,8 @@ Load_log_event::Load_log_event(THD *thd_
> >  {
> >    time_t end_time;
> >    time(&end_time);
> > -  exec_time = (ulong) (end_time  - thd_arg->start_time);
> > +  DBUG_ASSERT(end_time >= thd_arg->real_start_time);
> > +  exec_time = (ulong) (end_time  - thd_arg->real_start_time);
> >    /* db can never be a zero pointer in 4.0 */
> >    db_len = (uint32) strlen(db);
> >    table_name_len = (uint32) strlen(table_name);
> > 
> > === modified file 'sql/sql_class.h'
> > --- a/sql/sql_class.h	2010-04-14 09:53:59 +0000
> > +++ b/sql/sql_class.h	2010-05-01 12:03:34 +0000
> > @@ -1414,6 +1414,9 @@ public:
> >    /* remote (peer) port */
> >    uint16 peer_port;
> >    time_t     start_time, user_time;
> > +  time_t     real_start_time;        /* The real start time of the
> > +                                      * query, not affected by
> > +                                      * TIMESTAMP session variable */
> >    // track down slow pthread_create
> >    ulonglong  prior_thr_create_utime, thr_create_utime;
> >    ulonglong  start_utime, utime_after_lock;
> > @@ -2008,19 +2011,14 @@ public:
> >    inline time_t query_start() { query_start_used=1; return start_time; }
> >    inline void set_time()
> >    {
> > -    if (user_time)
> > -    {
> > -      start_time= user_time;
> > -      start_utime= utime_after_lock= my_micro_time();
> > -    }
> > -    else
> > -      start_utime= utime_after_lock= my_micro_time_and_time(&start_time);
> > +    start_utime= utime_after_lock=
> my_micro_time_and_time(&real_start_time);
> > +    start_time = user_time ? user_time : real_start_time;
> >    }
> > -  inline void	set_current_time()    { start_time= my_time(MY_WME); }
> > +  inline void	set_current_time() { real_start_time= start_time=
> my_time(MY_WME); }
> >    inline void	set_time(time_t t)
> >    {
> >      start_time= user_time= t;
> > -    start_utime= utime_after_lock= my_micro_time();
> > +    set_time();
> >    }
> >    void set_time_after_lock()  { utime_after_lock= my_micro_time(); }
> >    ulonglong current_utime()  { return my_micro_time(); }
> > 
> > text/bzr-bundle type attachment
> > (bzr/zhenxing.he@stripped)
> > # Bazaar merge directive format 2 (Bazaar 0.90)
> > # revision_id: zhenxing.he@stripped
> > # target_branch: file:///media/sda3/work/mysql/bzrwork/b52704/5.1-\
> > #   bugteam/
> > # testament_sha1: fe802bc5affef390af9d6cb3c9a202abe51c0cd9
> > # timestamp: 2010-05-01 20:03:47 +0800
> > # base_revision_id: svoj@stripped
> > # 
> > # Begin bundle
> > IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWW1UD+oAAwbfgGQwWXf///fi
> > XrC////6YAcLnznd2ihYrBR0pd3AKuElAmpT00T8jVN5Ub2inqntTap5NR6hkyDIYjQGgSRImU/T
> > RqeppRgTNT0gZDCMEwEaAGg4aaZGIwmmAhgE0wjBMTIaZGhoBIiImjUxGTTU0eoeppoxGT1BkAAN
> > MjQOGmmRiMJpgIYBNMIwTEyGmRoaASSEwhNGIAmjQFPTFPUzKZkEAAPTUnmR0Z6Km3Uy/zE47e/q
> > PjZblyS/vU0cPm3k1MCpl/il5HwQEYR6oxhpsbOrqvo+nI2uknX5o5AMBxOZ6F+8ZKnfWgzDbBsP
> > z9kZMdcJmgzcVq0043vNTNIm22uCaS8iWQ2uLt0HEat2pXLUZppE01itU1uIvZYz8aCdJP/Y+Owo
> > QOS7lLUf3UOwYiwZO/1xfI/hi4LOf+szqgsPcQJvvGHBW4zmo0UH2xiM7xrNlA9M81qWunTjZf+3
> > O1YxZk5DJDxmLm66jQA7DDjeAPutUefhBp2jFvxIEEfK7cyPnhg1NcXrXtVa8qwzoVgGomoCzlxm
> > wkoJlK8gp7e1bCaPUJaAwQM2/8fIsecVY9mpQlp2rklLep04BbqSslCoKRbYErqHnGUFUhBJyJjA
> > Q9hoNjixwmUKGy4wA5q9t4GVvHkqqYBVRcB5LLhMmBtTLs+NrVP1FkXG1EyuUq1a11Y0Ayx9uChG
> > TxXlWFief6TJDkqLC9pyrEqhSQTaDnqweTxI0fle/G+lIykiPu8DNYrgOcChOwzWq63MKUhw5gIA
> > XLaaRxgMOrW0eXk+0CBtVDXX+U5Ro0cuNAIWCK3B1ie2tUsNCxhUSIjSN6M7kxEyHeYNKca8uo99
> > AN0sPlfNWb7z0JlWJZlUTCqqgrYwYqNvGudTnpmNMOS0y2p9RIyECQ+Q4ySClxYch9+nRA9AKiMr
> > WrdWwnOc4s8D5SroTN/c4NQ8oWjzrSL7tFW/SXw1bDVJ9VRdDMZByYkM65mzJWw85eQ9kTMWTKy5
> > LPeYO0hM7jGBsZKmJzMkYZITC4q1e6RVFlFbfzib5kbftKbIS4GshyVKEweR4XcR5XK3kN8oyOD0
> > Lh1zSJdlIxFZ5AmebxBAHOLsIzDTb+p0Z7jpr1ECjrMeIX76xdIvjgFIiVDi/ZAUkvcUsh+j+Ymy
> > HOjc1kVDjLZTI3NU6vhZRrAkW3GDYVkmJalljET0RjCTjOMxCnpW/y+kDyD4Eaq8ysHI14hgipCp
> > Wm+r9AX1RpTN5eWUCAWIoIPPWj7kyicecobTzLRx5nFAzFbP1J3fMLdS3alVEVvakNrDUBHeQtPa
> > bgznF8C0PFGA8s+V3wMVBsJO5+CHViVlzwmnI1qLlBksIcr46FifE8zW4rREHEqVBCBUBMHowCxZ
> > mX0Tjcp3LTWXtKDs1kk7jqSiBb9b/CptsMb32gubW6dZe8c6WJgG4kTNLLrtZyKy9BVl6FUXIih0
> > fmXIUvUHTQ5PTMY4tKXVP5mmcfEO1EUyYTxG7CJauIUtot5jiEQaxwcEOQauUXC1GybcEbkugxHj
> > cRhxDCviMC0ClXorPqa856SlaVQSqNEFJW8lK/ZzlDM4ODzeDCHmoDiT+JFpJkn5FpP+CqhUqjXu
> > XYDfZwM6fzCbvAggPNpINTQ9dyDn7E9RMy2qxA442PA3KwU6oTaZalKXAvQIzGxTaLREagEcxd3I
> > wx59NrILZ93XJ9ljDAnLQcEGPrUXd4GgMy1rlNwDUNaJ9RvCwGQ5IijpTgb6cMhjEwYiZabovjvc
> > s6+jzbem7KMs9y2LbWJQrU4gHBgzGGiBqrHWoRkRiri1UWK1dsvy3PF0rBJgKhpWiyzYnCMARFts
> > BAxDnEsVKrXl14T5rKbAMiRRhXpycItpRDZye1siHI5kbklWAdcCuSVaI1h7XreuAafZRBl6FmXD
> > hxipFUFvMnHAnNMYT6lwcdmKeZ1egrUVaSWYWnhtFgjcxhCIdJ0Mf0/0Y6onGwn6+hxsZPggulcr
> > lgdZp4R0CxrEFZRkMwmPdzBBnbENVQZnh0TRwvRH6/vBXLKtGSwGAgWhvFyrDw7lsa1zndow5yfo
> > ReBpTYmG/KMwD2fy2Ij6iTupNkY4gNC7FZZdcGPAyFEdwQiQgdoRQ57kixIKlVLciGzryBZMccTA
> > De8x3mRBVttNAd5twoV0IxIyisYd2yQZ59rQL48nTist3gXXJklnjsvDBnqekeLWiid0qtX98UFa
> > ByqeHPWoDBplEtMrCI5Veqozh2jMQQlwJxmcyGSCj61lbuVu+tPJqMAYRzVSVJ/xdyRThQkG1UD+
> > oA==
> > 
> > 
> 
> 
> 


Thread
bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453) Bug#52704He Zhenxing1 May
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453)Bug#52704Luís Soares4 May
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453)Bug#52704He Zhenxing12 May
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453)Bug#52704Libing Song12 May
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3453)Bug#52704Libing Song13 May