Hello Marc,
Guilhem Bichot a écrit, Le 25.08.2009 16:25:
> 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.
for what it's worth, I tested how the Event Scheduler behaves when it
finds (via table_check_intact()) that mysql.event has been changed:
I dropped mysql.event, recreated it (but with an INT column changed to
FLOAT), then restarted mysqld and:
090826 10:50:08 [ERROR] Incorrect definition of table mysql.event:
expected column 'interval_value' at position 5 to have type int(11),
found type float.
090826 10:50:08 [ERROR] Event Scheduler: An error occurred when
initializing system tables. Disabling the Event Scheduler.
090826 10:50:08 [Note]
/home/mysql_src/bzrrepos/mysql-azalea-perfschema/sql/mysqld: ready for
connections.
so mysqld starts with Event scheduler disabled.
