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

Proposed by Oleg Tsarev on 2011-10-08
Status: Merged
Approved by: Stewart Smith on 2011-10-08
Approved revision: 322
Merged at revision: 322
Proposed branch: lp:~tsarev/percona-server/5.1_fix_bug_838725
Merge into: lp:percona-server/5.1
Prerequisite: lp:~tsarev/percona-server/5.1_fix_bug_838725_cumulative
Diff against target: 0 lines
To merge this branch: bzr merge lp:~tsarev/percona-server/5.1_fix_bug_838725
Reviewer Review Type Date Requested Status
Laurynas Biveinis 2011-10-08 Approve on 2011-10-08
Alexey Kopytov 2011-10-08 Pending
Review via email: mp+78695@code.launchpad.net

This proposal supersedes a proposal from 2011-10-05.

Description of the change

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

To post a comment you must log in.
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

** 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) wrote : Posted in a previous version of this proposal

 > ** 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: Resubmit
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

> 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) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

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

review: Needs Fixing
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

LGTM

review: Approve

LGTM if all individual MPs are approved

review: Approve

Preview Diff

Empty

Subscribers

People subscribed via source and target branches