Code review comment for lp:~akopytov/percona-xtrabackup/bug1079700-2.0

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Hi Alexey -

> > - The MLOG_FILE_DELETE bits need reconcilation and/or explanation
> > http://bugs.mysql.com/bug.php?id=43948 and
>
> The explanation is that you did not port the patch for bug #43948 to
> innodb56.patch. Now that I have patched recv_apply_hashed_log_recs(),
> the fix for bug 43948 in PS and innodb51.patch/innodb55.patch is redundant.

Yes, I was wondering about other than innodb56.patch patches. The fact that I didn't patch innodb56.patch is/was https://bugs.launchpad.net/percona-xtrabackup/+bug/1121072. So if I understand correctly, now they will carry the both fixes and either fix may be reverted. And there is no point to spend effort to actually do this.

> > - This might be outside the scope of the current fix, but since
> > you are backporting the mysqldump backup verifier, I am
> > mentioning it here. If there's a bug/blueprint open for this,
> > please point me there: mysqldump verification probably would miss
> > secondary index tree corruptions, wouldn't it?
> >
>
> Both mysqldump and CHECKSUM TABLE verification are identical for InnoDB.
> CHECKSUM TABLE basically does "SELECT * FROM table" internally and
> calculates checksums for all values in all rows. So corruption in
> secondary index pages will also go unnoticed.

I didn't mention CHECKSUM TABLE anywhere in my comment for this very reason. So do we want to do something re. secondary index tree verifications?

review: Approve

« Back to merge proposal