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

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

Sergei,

   Summary of our IRC discussion for the record:
   - test case for bug #1155475
   - unified return types in sql/handler.cc
   - error on bogus filename in innobase_purge_archive_logs()

   Other comments:

   - archived_file_no / next_archived_file_no are now LSNs rather that
     ordinal file numbers. Which means their type should be lsn_t, not
     ulint. The same for 'file_no' argument f
     log_archived_file_name_gen().

   - the assertion and sprintf() in log_archived_file_name_gen() seem to
     assume the maximum length for a decimal representation of file_no
     is 16 characters, but it is a 64-bit integer, so the maximum length
     is 20 characters.

   - the following line in log_archived_file_name_gen() still is still
     longer than 80 chars:

: sprintf(buf + dirnamelen, IB_ARCHIVED_LOGS_PREFIX "%016lu", (ulong) file_no);

   - the following line in log_group_archive() is still longer than 80
     chars:

: n_files * (group->file_size - LOG_FILE_HDR_SIZE),

review: Needs Fixing

« Back to merge proposal