Code review comment for lp:~sergei.glushchenko/percona-server/ps56-univ-log-archive

Revision history for this message
Alexey Kopytov (akopytov) wrote :

   - multiple lines violate the 80 chars limit, try:

: bzr diff -c-1 | grep -0 '^+ ' | awk 'length > 82'

   - there's still no reply in http://bugs.mysql.com/bug.php?id=68635
     We obviously cannot afford similar changes for every new statement
     type, so we should do something about it. What about adding
     performance_schema_max_statement_classes = 167 to
     mysql-test/include/default_mysqld.cnf?

   - the changes related to digest values is another "gift" from
     performance schema. Not sure what to do about it. Probably just
     "--replace-column ... <digest>" in the corresponding tests?

  - please use OS_FILE_MAX_PATH instead of hard-coded (and
    inconsistent) constants in log_group_archive() and
    log_group_recover_from_archive_file(). I don't think it then makes
    sense to assert on sufficient buffer space, as a lot of things
    would break if paths are longer than that. But you can keep the
    assertion if you like. The rest of InnoDB code seems to assume that
    never happens, but I agree we should fail *if* it does.

review: Needs Fixing

« Back to merge proposal