Merge lp:~tsarev/percona-server/5.1_fix_bug_838725 into lp:percona-server/5.1

[Oleg Tsarev]

Cumulative fix related to bug #838725

Clean Jenkins result: http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-param/168/

Fixed following:

FAILED TESTS:
1. main.endspace main.index_merge_innodb main.rowid_order_innodb main.type_bit_innodb
https://code.launchpad.net/~tsarev/percona-server/5.1_fix_bug_832528/+merge/78687

2. main.percona_innodb_kill_idle_trx main.percona_innodb_kill_idle_trx_locks
https://code.launchpad.net/~tsarev/percona-server/fix_for_kill_idle_trx_test/+merge/78691

3. main.percona_server_variables_debug main.percona_server_variables_release
https://code.launchpad.net/~tsarev/percona-server/fix_for_kill_idle_trx_test/+merge/78691
https://code.launchpad.net/~tsarev/percona-server/5.1_fix_bug_860416/+merge/78692

FAILED TESTCASES (JUST DEBUG):
1. rpl.rpl_binlog_errors
https://code.launchpad.net/~tsarev/percona-server/5.1_fix_bug_794790/+merge/78694

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-centos/1/

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-centos/8/

[Oleg Tsarev (tsarev)]

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-centos/9/

[Alexey Kopytov (akopytov)]

** after your changes have_innodb_plugin.inc is functionally identical to have_innodb.inc. Which means there's no need for s/have_innodb/have_innodb_plugin/ in percona tests
** mtr is just a symlink to mysql-test-run.pl, there's no need to duplicate diff changes
** please revert the "Running test cases" changes. they might be good for debugging, but they don't carry any functional changes, so they can only increase maintenance cost.
** MySQL bug #62340 reported by you is invalid. The difference in results has been introduced by your changes in Percona Server, it has nothing to do with upstream.
** main.index_merge_innodb, main.rowid_order_innodb, main.type_bit_innodb and main.endspace result differences look fishy. I understand that there may be a difference in results between builtin InnoDb and plugin/XtraDB. But those tests used to pass with XtraDB in older Percona Server releases. Isn't it something we should look into instead of just updating .result files? In other words, what's changed in those tests or XtraDB between Percona Server 5.1.55 and 5.1.58 that leads to results being different?
** I would like a link to a Jenkins build on all slaves, not just the centos ones.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

 > ** after your changes have_innodb_plugin.inc is functionally identical to have_innodb.inc. Which means there's no need for s/have_innodb/have_innodb_plugin/ in percona tests

ACK

 > ** mtr is just a symlink to mysql-test-run.pl, there's no need to duplicate diff changes

Unfortunatelly - not a symlink. You can check this by call "make all" and see to the Percona_Server-*-*/mysql-test/".

 > ** please revert the "Running test cases" changes. they might be good for debugging, but they don't carry any functional changes, so they can only increase maintenance cost.

This output very good for analyzing Jenkins logs. I don't see the big increase of maintenance time.
This is useful for me at least. Can we save this?

> ** MySQL bug #62340 reported by you is invalid. The difference in results has been introduced by your changes in Percona Server, it has nothing to do with upstream.

1) Bug #62340 is open yet. Where did you see this bug as "Invalid"?
2) Query " select text1, length(text1) from t1 where text1='teststring' or text1 like
'teststring_%';" has no guarantees for output order. I fix upstream test by "order by"
3) This changes to Percona-Server not is "my". This test doesn't work long time according to bug #838725, and right now start work again.

> ** main.index_merge_innodb, main.rowid_order_innodb, main.type_bit_innodb and main.endspace result differences look fishy. I understand that there may be a difference in results between builtin InnoDb and plugin/XtraDB. But those tests used to pass with XtraDB in older Percona Server releases. Isn't it something we should look into instead of just updating .result files? In other words, what's changed in those tests or XtraDB between Percona Server 5.1.55 and 5.1.58 that leads to results being different?

This is a good question, but I hasn't answer to it. May be ask Yasufumi for that? I will write letter to him.

> ** I would like a link to a Jenkins build on all slaves, not just the centos ones.

When all slaves will work fine, I will run tests on all platforms.

review: Needs Resubmitting

[Alexey Kopytov (akopytov)]

On 05.09.11 19:13, Oleg Tsarev wrote:
> > ** mtr is just a symlink to mysql-test-run.pl, there's no need to duplicate diff changes
>
> Unfortunatelly - not a symlink. You can check this by call "make all" and see to the Percona_Server-*-*/mysql-test/".
>

It is created as a symlink when building from a Bazaar tree. Anyway,
both mtr and mysql-test-run are essentially autogenerated files created
from mysql-test-run.pl (it's just that in tarballs they are copied
rather than symlinked).

So what we should do instead is to remove them when applying the patches
to the tarball. Then they will be created as symlinks to
mysql-test-run.pl when building. It is better than replicating changes
in 3 different files.

Can you report a separate bug for that and submit a separate fix?

> > ** please revert the "Running test cases" changes. they might be good for debugging, but they don't carry any functional changes, so they can only increase maintenance cost.
>
> This output very good for analyzing Jenkins logs. I don't see the big increase of maintenance time.
> This is useful for me at least. Can we save this?
>

You can see the suite names from the logs themselves. Each test is
prefixed with the suite name.

[Alexey Kopytov (akopytov)]

main.index_merge_innodb, main.rowid_order_innodb, main.type_bit_innodb and main.endspace result differences are due to bug53761.patch. So the related changes should be moved to bug53761.patch rather than mysql-test.diff.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

> main.index_merge_innodb, main.rowid_order_innodb, main.type_bit_innodb and
> main.endspace result differences are due to bug53761.patch. So the related
> changes should be moved to bug53761.patch rather than mysql-test.diff.

ACK

[Laurynas Biveinis (laurynas-biveinis)]

LGTM.

Please please please in the future submit separate MPs for individual issues. It is not important that some of them depend on others, it makes reviews much more manageable.

review: Approve

[Alexey Kopytov (akopytov)]

  - merge bug62340.patch into bug53761.patch
  - remove changes related to log_slow_admin_statements

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

LGTM

review: Approve

[Laurynas Biveinis (laurynas-biveinis)]

LGTM if all individual MPs are approved

review: Approve