Hello Marc,
Marc Alff a écrit, Le 19.08.2009 10:06:
> #At file:///home/malff/BZR_TREE/mysql-azalea-perfschema/ based on
> revid:marc.alff@stripped
>
> 2852 Marc Alff 2009-08-19
> WL#2360 PERFORMANCE SCHEMA
>
> Implemented calls to table_check_intact() for performance schema
> tables, per code review comments.
Thanks, patch is fine except one decision which I wonder about:
> +/**
> + Check that the performance schema tables
> + have the expected structure.
> + Discrepancies are written in the server log,
> + but are not considered fatal, so this function does not
> + return an error code:
> + - some differences are compatible, and should not cause a failure
> + - some differences are not compatible, but then the DBA needs an operational
> + server to be able to DROP+CREATE the tables with the proper structure,
> + as part of the initial server installation or during an upgrade.
> +*/
> +void check_performance_schema()
> +{
The decision to let P_S enabled if tables don't match...
This half-defeats the purpose of the check. DBA may not look in the
error log all the time; monitoring application may start querying P_S
right after an upgrade, server crashes in P_S code may then immediately
happen.
If a mismatch in tables allowed the server to start but with P_S disabled:
- the second case above ("some differences are not compatible, but then
the DBA needs an operational server" etc) is still OK: the server is
functional enough to do DROP+CREATE on P_S tables.
- the first case ("some differences are compatible, and should not cause
a failure"): I see in table_check_intact() that it already makes efforts
to be tolerant; we can see those comments:
Let's check column definitions. If a column was added at
the end of the table, then we don't care much since such change
is backward compatible.
...
Name changes are not fatal, we use ordinal numbers to access
columns.
Still this can be a sign of a tampered table, output an error
to the error log.
...
Generally, if column types don't match, then something is
wrong.
However, we only compare column definitions up to the
length of the original definition, since we consider the
following definitions compatible:
1. DATETIME and DATETIM
2. INT(11) and INT(11
3. SET('one', 'two') and SET('one', 'two', 'more')
For SETs or ENUMs, if the same prefix is there it's OK to
add more elements - they will get higher ordinal numbers and
the new table definition is backward compatible with the
original one.
...
So I would rather say that if table_check_intact() said "not OK", P_S
should trust that something may well go wrong, and we should not the
user run into it, so we should let the server run without P_S.
What do you think?
--
Mr. Guilhem Bichot <guilhem@stripped>
Sun Microsystems / MySQL, Lead Software Engineer
Bordeaux, France
www.sun.com / www.mysql.com
