Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-27912

--source include/restart_mysqld.inc fails in Spider test suite

Details

    Description

      --source include/restart_mysqld.inc results in a test failure in Spider suites, with no output from mysqltest. Putting the following MTR test case to storage/spider/mysql-test/spider/bugfix/t reproduces the bug.

      --source include/restart_mysqld.inc
      

      This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

      The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.

      Attachments

        Issue Links

          Activity

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) created issue -
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Field Original Value New Value
            Description {code:sql} {{

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. An

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. An

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. An MTR test case for the reproduction:

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Component/s Storage Engine - Spider [ 10132 ]
            Component/s Tests [ 10800 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. An MTR test case for the reproduction:

            {code:sql}
            INSTALL PLUGIN spider SONAME 'ha_spider.so';
            --source include/restart_mysqld.inc
            UNINSTALL PLUGIN spider;
            {code}
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            I will start working on the issue once MDEV-22979 is fixed.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - I will start working on the issue once MDEV-22979 is fixed.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites, with no output from MTR. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites, with no output from MTR. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites, with no output from mysqltest. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            julien.fritsch Julien Fritsch made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            serg Sergei Golubchik made changes -
            serg Sergei Golubchik made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Assignee Nayuta Yanagisawa [ JIRAUSER47117 ] Alexey Botchkov [ holyfoot ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Fix Version/s 10.2 [ 14601 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Status Open [ 1 ] Confirmed [ 10101 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Assignee Alexey Botchkov [ holyfoot ] Nayuta Yanagisawa [ JIRAUSER47117 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Status Confirmed [ 10101 ] In Progress [ 3 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -

            Deleting storage/spider/mysql-test/spider/bugfix/my.cnf fixes the problem.

            Deleting my_1_1.cnf from the file result in a hang:

            --- a/storage/spider/mysql-test/spider/bugfix/my.cnf
            +++ b/storage/spider/mysql-test/spider/bugfix/my.cnf
            @@ -1,2 +1 @@
             !include include/default_mysqld.cnf
            -!include my_1_1.cnf
            

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - Deleting storage/spider/mysql-test/spider/bugfix/my.cnf fixes the problem. Deleting my_1_1.cnf from the file result in a hang: --- a/storage/spider/mysql-test/spider/bugfix/my.cnf +++ b/storage/spider/mysql-test/spider/bugfix/my.cnf @@ -1,2 +1 @@ !include include/default_mysqld.cnf -!include my_1_1.cnf

            Putting the empty file storage/spider/mysql-test/spider/bugfix/my.cnf still cause a hang.

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - Putting the empty file storage/spider/mysql-test/spider/bugfix/my.cnf still cause a hang.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Description {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites, with no output from mysqltest. Putting the following MTR test case to {{storage/spider/mysql-test/spider/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            {{--source include/restart_mysqld.inc}} results in a test failure in Spider suites, with no output from mysqltest. Putting the following MTR test case to {{storage/spider/mysql-test/spider/bugfix/t}} reproduces the bug.

            {code:sql}
            --source include/restart_mysqld.inc
            {code}

            This doesn't happen at least in the main suite and the include file is used elsewhere. So, I expect that the problem is Spider-specific.

            The Spider hangs on the server startup on 10.4+ (MDEV-22979) and thus the failure is expected on 10.4+. However, even on 10.2, the test fails. This is possibly due to a different reason.
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Priority Major [ 3 ] Critical [ 2 ]

            As elenst said (in a private communication), this is due to the non-conventional server numbering of the Spider test suite.

            restart_mysqld.inc writes an *.expect file so that MTR wouldn't panic when the server is down. The file has a server number in it. In Spider case, it's written as mysqld.1.expect, while the servers have double-numbering, e.g. 1.1.

            A workaround proposed by elenst is manually doing some part of what restart_mysqld.inc does like the following:

            --exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.1.expect
            --shutdown_server
            --source include/wait_until_disconnected.inc
            --exec echo "restart" > $MYSQLTEST_VARDIR/tmp/mysqld.1.1.expect
            --enable_reconnect
            --source include/wait_until_connected_again.inc
            

            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) added a comment - - edited As elenst said (in a private communication), this is due to the non-conventional server numbering of the Spider test suite. restart_mysqld.inc writes an *.expect file so that MTR wouldn't panic when the server is down. The file has a server number in it. In Spider case, it's written as mysqld.1.expect, while the servers have double-numbering, e.g. 1.1. A workaround proposed by elenst is manually doing some part of what restart_mysqld.inc does like the following: --exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.1.expect --shutdown_server --source include/wait_until_disconnected.inc --exec echo "restart" > $MYSQLTEST_VARDIR/tmp/mysqld.1.1.expect --enable_reconnect --source include/wait_until_connected_again.inc
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            nayuta-yanagisawa Nayuta Yanagisawa (Inactive) made changes -
            Status In Progress [ 3 ] Stalled [ 10000 ]
            ycp Yuchen Pei made changes -
            Assignee Nayuta Yanagisawa [ JIRAUSER47117 ] Yuchen Pei [ JIRAUSER52627 ]
            ycp Yuchen Pei added a comment -

            The workaround became storage/spider/mysql-test/spider/bugfix/include/restart_spider.inc.

            However the filename is misleading and should better be something like restart_mysqld_spider.inc.

            Also, it misses some features from restart_mysqld.inc, including parameters like $restart_parameters or $shutdown_timeout. To fix that, we can have some options:

            1. Update the spider version of restart_mysqld.inc. This will result in more code duplication
            2. Update the restart_mysqld.inc by adding an expect_file_name parameter which is simpler and cleaner:

            diff --git a/mysql-test/include/shutdown_mysqld.inc
                b/mysql-test/include/shutdown_mysqld.inc
            index db0cfb82c68..0a5b9f4fb40 100644
            --- a/mysql-test/include/shutdown_mysqld.inc
            +++ b/mysql-test/include/shutdown_mysqld.inc
            @@ -24,6 +24,10 @@ if ($rpl_inited)
             # Write file to make mysql-test-run.pl expect the "crash", but don't
                start it
             --let $_server_id= `SELECT @@server_id`
             --let $_expect_file_name=
                $MYSQLTEST_VARDIR/tmp/mysqld.$_server_id.expect
            +if ($expect_file_name)
            +{
            +  -- let $_expect_file_name=$expect_file_name
            +}
             --exec echo "wait" > $_expect_file_name
             
             # Avoid warnings from connection threads that does not have time to
                exit

            ycp Yuchen Pei added a comment - The workaround became storage/spider/mysql-test/spider/bugfix/include/restart_spider.inc . However the filename is misleading and should better be something like restart_mysqld_spider.inc . Also, it misses some features from restart_mysqld.inc , including parameters like $restart_parameters or $shutdown_timeout . To fix that, we can have some options: 1. Update the spider version of restart_mysqld.inc . This will result in more code duplication 2. Update the restart_mysqld.inc by adding an expect_file_name parameter which is simpler and cleaner: diff --git a/mysql-test/include/shutdown_mysqld.inc b/mysql-test/include/shutdown_mysqld.inc index db0cfb82c68..0a5b9f4fb40 100644 --- a/mysql-test/include/shutdown_mysqld.inc +++ b/mysql-test/include/shutdown_mysqld.inc @@ -24,6 +24,10 @@ if ($rpl_inited) # Write file to make mysql-test-run.pl expect the "crash", but don't start it --let $_server_id= `SELECT @@server_id` --let $_expect_file_name= $MYSQLTEST_VARDIR/tmp/mysqld.$_server_id.expect +if ($expect_file_name) +{ + -- let $_expect_file_name=$expect_file_name +} --exec echo "wait" > $_expect_file_name # Avoid warnings from connection threads that does not have time to exit
            ycp Yuchen Pei made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            ycp Yuchen Pei added a comment - - edited

            Hi serg, elenst suggested that I send this to you for review, and you may suggest someone else to review it. Here's the (tiny) patch:

            https://github.com/MariaDB/server/commit/fb7902b116f

            ycp Yuchen Pei added a comment - - edited Hi serg , elenst suggested that I send this to you for review, and you may suggest someone else to review it. Here's the (tiny) patch: https://github.com/MariaDB/server/commit/fb7902b116f
            ycp Yuchen Pei made changes -
            Assignee Yuchen Pei [ JIRAUSER52627 ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            ycp Yuchen Pei made changes -
            Affects Version/s 10.3 [ 22126 ]
            Affects Version/s 10.4 [ 22408 ]
            Affects Version/s 10.5 [ 23123 ]
            Affects Version/s 10.6 [ 24028 ]
            Affects Version/s 10.8 [ 26121 ]
            Affects Version/s 10.9 [ 26905 ]
            Affects Version/s 10.10 [ 27530 ]
            Affects Version/s 10.11 [ 27614 ]
            Affects Version/s 11.0 [ 28320 ]
            Affects Version/s 10.2 [ 14601 ]

            The real problem is that shutdown_mysqld.inc (and, likely, other .inc files too) use @@server_id while mtr uses group suffix. If .inc files would be using the correct file name, spider tests wouldn't need to specify the expect file manually.

            I suggest the following fix instead

            ---let $_server_id= `SELECT @@server_id`
            ---let $_expect_file_name= $MYSQLTEST_VARDIR/tmp/mysqld.$_server_id.expect
            +--let $_expect_file_name= `select regexp_replace(@@tmpdir, '^.*/','')`
            +--let $_expect_file_name= $MYSQLTEST_VARDIR/tmp/$_expect_file_name.expect
            

            that seems to work correctly for your spider test case and for other test cases too.

            Perhaps this should be done in all other include files that use $_expect_file_name ?

            Note, after this change innodb/t/temporary_table.test will fail, because it changes tmpdir to /dev/null/nonexistent. Can be fixed with

            ---exec echo "restart: --tmpdir=/dev/null/nonexistent" > $_expect_file_name
            +--exec echo "restart: --tmpdir=/dev/null/$MYSQL_TMP_DIR" > $_expect_file_name
            

            (and also add_suppression() at the top of the file needs to be adjusted)

            serg Sergei Golubchik added a comment - The real problem is that shutdown_mysqld.inc (and, likely, other .inc files too) use @@server_id while mtr uses group suffix. If .inc files would be using the correct file name, spider tests wouldn't need to specify the expect file manually. I suggest the following fix instead ---let $_server_id= `SELECT @@server_id` ---let $_expect_file_name= $MYSQLTEST_VARDIR/tmp/mysqld.$_server_id.expect +--let $_expect_file_name= `select regexp_replace(@@tmpdir, '^.*/','')` +--let $_expect_file_name= $MYSQLTEST_VARDIR/tmp/$_expect_file_name.expect that seems to work correctly for your spider test case and for other test cases too. Perhaps this should be done in all other include files that use $_expect_file_name ? Note, after this change innodb/t/temporary_table.test will fail, because it changes tmpdir to /dev/null/nonexistent . Can be fixed with ---exec echo "restart: --tmpdir=/dev/null/nonexistent" > $_expect_file_name +--exec echo "restart: --tmpdir=/dev/null/$MYSQL_TMP_DIR" > $_expect_file_name (and also add_suppression() at the top of the file needs to be adjusted)
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Yuchen Pei [ JIRAUSER52627 ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            julien.fritsch Julien Fritsch made changes -
            ycp Yuchen Pei added a comment - - edited

            Thanks for the review serg, PTAL thanks: https://github.com/MariaDB/server/commit/0548a185a1e

            ycp Yuchen Pei added a comment - - edited Thanks for the review serg , PTAL thanks: https://github.com/MariaDB/server/commit/0548a185a1e
            ycp Yuchen Pei made changes -
            Assignee Yuchen Pei [ JIRAUSER52627 ] Sergei Golubchik [ serg ]
            Status Stalled [ 10000 ] In Review [ 10002 ]

            0548a185a1e looks good, thanks. Ok to push

            serg Sergei Golubchik added a comment - 0548a185a1e looks good, thanks. Ok to push
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Yuchen Pei [ JIRAUSER52627 ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.7 [ 24805 ]
            ycp Yuchen Pei made changes -
            Fix Version/s 10.9 [ 26905 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            ycp Yuchen Pei added a comment - - edited

            I got back to this task today and tested the patch for 10.3-11.1 (hopefully for the last time). I tried to push to 10.3 but got rejected:

            remote: error: GH006: Protected branch update failed for refs/heads/10.3.        
            remote: error: You're not authorized to push to this branch. Visit https://docs.github.com/articles/about-protected-branches/ for more information.        
            To ssh://ssh.github.com/MariaDB/server.git
             ! [remote rejected]         bb-10.3-mdev-27912 -> 10.3 (protected branch hook declined)
            error: failed to push some refs to 'ssh://ssh.github.com/MariaDB/server.git'
            

            The CI displays a green checkmark[1]. I wonder whether this means the branch is locked. serg Shall I try pushing to 10.4 instead?
            [1] https://github.com/MariaDB/server/commit/88a589d4efa

            ycp Yuchen Pei added a comment - - edited I got back to this task today and tested the patch for 10.3-11.1 (hopefully for the last time). I tried to push to 10.3 but got rejected: remote: error: GH006: Protected branch update failed for refs/heads/10.3. remote: error: You're not authorized to push to this branch. Visit https://docs.github.com/articles/about-protected-branches/ for more information. To ssh://ssh.github.com/MariaDB/server.git ! [remote rejected] bb-10.3-mdev-27912 -> 10.3 (protected branch hook declined) error: failed to push some refs to 'ssh://ssh.github.com/MariaDB/server.git' The CI displays a green checkmark [1] . I wonder whether this means the branch is locked. serg Shall I try pushing to 10.4 instead? [1] https://github.com/MariaDB/server/commit/88a589d4efa
            ycp Yuchen Pei added a comment -

            Pushed to 10.4, and in time it will be merged to newer versions.

            10.5, 10.6 have merge conflicts, and reference commits were pushed to bb-10.

            {5,6}

            -mdev-27912.

            ycp Yuchen Pei added a comment - Pushed to 10.4, and in time it will be merged to newer versions. 10.5, 10.6 have merge conflicts, and reference commits were pushed to bb-10. {5,6} -mdev-27912.
            ycp Yuchen Pei made changes -
            Fix Version/s 10.4.29 [ 28510 ]
            Fix Version/s 10.5.20 [ 28512 ]
            Fix Version/s 10.6.13 [ 28514 ]
            Fix Version/s 10.8.8 [ 28518 ]
            Fix Version/s 10.9.6 [ 28520 ]
            Fix Version/s 10.10.4 [ 28522 ]
            Fix Version/s 10.11.3 [ 28524 ]
            Fix Version/s 11.1.0 [ 28705 ]
            Fix Version/s 11.0.2 [ 28706 ]
            Fix Version/s 10.7.8 [ 28515 ]
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.8 [ 26121 ]
            Fix Version/s 10.9 [ 26905 ]
            Fix Version/s 10.10 [ 27530 ]
            Fix Version/s 10.11 [ 27614 ]
            Fix Version/s 11.0 [ 28320 ]
            Fix Version/s 11.1 [ 28549 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]

            People

              ycp Yuchen Pei
              nayuta-yanagisawa Nayuta Yanagisawa (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.