List:Commits« Previous MessageNext Message »
From:Marc Alff Date:July 21 2010 3:26pm
Subject:Re: bzr commit into mysql-trunk branch (aelkin:3073) WL#3656
View as plain text  
Hi Andrei

On 7/16/10 9:05 AM, Andrei Elkin wrote:
> #At file:///home/andrei/MySQL/BZR/2a-23May/WL/5.5-trunk-wl3656-p_s-sss/ based on
> revid:sunanda.menon@stripped
>
>   3073 Andrei Elkin	2010-07-16
>        WL#3656 Performance Schema table for SHOW SLAVE STATUS.
>
>        The patch implements a new
>        table to the existing P_S collection.
>        The new interface allows to fetch individual items and handle them according
>        to their mysql types (see show create peformance_schema.SLAVE_STATUS).
>        Output of the old SHOW SLAVE STATUS is identical to one of
>         SELECT * from peformance_schema.SLAVE_STATUS.
>        In particular if active_mi is NULL both outputs are NULL set.
>
>        A test checks the mentioned basic properties.
>
>        CAVEAT: building fail at embedded library testing. There is a bug in PS build
> scheme allowing
>        PS linking into libmysqld that should not take place (Marc_Alff is alerted and
> works on the issue).
>       @ mysql-test/suite/rpl/r/rpl_ps_slave_status.result
>          new test results.
>       @ mysql-test/suite/rpl/t/rpl_ps_slave_status.test
>          basic functionalilty of performance_schema.SLAVE_STATUS as comparsion with
>          SHOW SLAVE STATUS to prove identical output.
>       @ scripts/mysql_system_tables.sql
>          adding static definition for the new P_S.SLAVE_STATUS table.
>       @ storage/perfschema/Makefile.am
>          adding h,cc files to build in static P_S.SLAVE_STATUS.
>       @ storage/perfschema/pfs_engine_table.cc
>          Extending PFS private global array of specific table shares with
>          one for the new table.
>       @ storage/perfschema/table_slave_status.cc
>          Implementation of classes for the new table.
>       @ storage/perfschema/table_slave_status.h
>          Declarations for the new table.
>
>      added:
>        mysql-test/suite/rpl/r/rpl_ps_slave_status.result
>        mysql-test/suite/rpl/t/rpl_ps_slave_status.test
>        storage/perfschema/table_slave_status.cc
>        storage/perfschema/table_slave_status.h
>      modified:
>        scripts/mysql_system_tables.sql
>        storage/perfschema/Makefile.am
>        storage/perfschema/pfs_engine_table.cc


For Worklogs, reviewing every commit is not practical: what is more 
effective is to push the code into a working branch for the project 
(mysql-next-mr-wl3656 in this case), fix build problems, test cases 
problems, etc.

Once the branch is working, only then a formal review can start.

Typically, I will do this with:
bzr diff --old mysql-next-mr --new mysql-next-mr-wl3656
so nothing will be missed, and the order and number of pushes does not 
matter ... you should not have to wait for me every time.

So, please push this code into mysql-next-mr-wl3656, and push the next 
patches directly there.

 From what I could see reading this patch:

- It is based on mysql-trunk (5.5). The code in next-mr (5.6) is 
slightly different because of later cleanups: for example, 
PFS_readonly_table is gone.

- When adding files to Makefile.am, don't forget CMakeLists.txt

- When adding new tables in the performance schema, please update 
mysql-test/suite/perfschema/include/start_server_common.inc, and adjust 
the test results in the test suite accordingly. Existing tests (schema, 
information_schema) will need to be adjusted as well.

- Please provide a basic ddl_slave_status.test and 
dml_slave_status.test. See the existing tests there, this is just 
boilerplate code for systematic coverage.

We probably will need to discuss how to implement the mapping between 
columns in the SLAVE_STATUS tables to values, but this can be done later 
and separately.

Overall, I think this patch is a good base.

I may have a fix for the embedded build issue, but I need the code in 
mysql-next-mr-wl3656 first to test it :)

Thanks for working on this.

Regards,
-- Marc

P.S.

Chris, see above, in case you want to document a "how to add a new table 
to the performance schema" HOWTO ...

Thread
bzr commit into mysql-trunk branch (aelkin:3073) WL#3656Andrei Elkin16 Jul
  • Re: bzr commit into mysql-trunk branch (aelkin:3073) WL#3656Marc Alff21 Jul