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

MTR tests fail when built without WSREP

Details

    Description

      When building with -DWITH_WSREP=OFF, files required for MTR tests are excluded and several tests fail. This is cause by a recent commit 7b44d0ba which attempted to solve MDEV-23230.

      Failures can be seen in GitLab CI, or by building/installing/testing in the fedora:latest Docker container with the -DWITH_WSREP=OFF CMake flag enabled.

      It appears this issue happened before (see !2196 and MDEV-28782) where WITH_WSREP=0 cause MTR tests to break.

      Perhaps there should be a CI build with WSREP/Galera disabled to catch this mistake sooner.

      Attachments

        Issue Links

          Activity

            cross Chris Ross added a comment - - edited

            I have no issues with the change if it's right. I'm not sure what the MTR tests are, and why they need the wsrep/galera includes if wsrep isn't to be used. Shouldn't the tests instead be changed to not use need them when building without WSREP, similar to what was done to fix MDEV-25625?

            cross Chris Ross added a comment - - edited I have no issues with the change if it's right. I'm not sure what the MTR tests are, and why they need the wsrep/galera includes if wsrep isn't to be used. Shouldn't the tests instead be changed to not use need them when building without WSREP, similar to what was done to fix MDEV-25625 ?

            cross wrote:

            Shouldn't the tests instead be changed to not use need them when building without WSREP, similar to what was done to fix MDEV-25625?

            Yes, agreed. If tests aren't actually using Galera/WSREP (and clearly if they're passing when it's not built… they're not ) then they shouldn't be including Galera/WSREP include files.

            dlenski Daniel Lenski (Inactive) added a comment - cross wrote: Shouldn't the tests instead be changed to not use need them when building without WSREP, similar to what was done to fix MDEV-25625 ? Yes, agreed. If tests aren't actually using Galera/WSREP (and clearly if they're passing when it's not built… they're not ) then they shouldn't be including Galera/WSREP include files.
            danblack Daniel Black added a comment -

            The test Robin tripped on Gitlab was missing include/wait_wsrep_ready.inc called from ./include/start_mysqld.inc (more widely used on many tests that restart the server) which has inbuilt tests which detects galera missing, which results in it being largely a no-op, but mtr can't tell that unless the file is there.

            Both wsrep.h and wsrep_on.h contain no-op macros needed in the without wsrep case. so hence https://github.com/MariaDB/server/pull/2417 which adds the two files to the Devel package. Thanks dlenski

            danblack Daniel Black added a comment - The test Robin tripped on Gitlab was missing include/wait_wsrep_ready.inc called from ./include/start_mysqld.inc (more widely used on many tests that restart the server) which has inbuilt tests which detects galera missing, which results in it being largely a no-op, but mtr can't tell that unless the file is there. Both wsrep.h and wsrep_on.h contain no-op macros needed in the without wsrep case. so hence https://github.com/MariaDB/server/pull/2417 which adds the two files to the Devel package. Thanks dlenski
            cross Chris Ross added a comment -

            Okay. Just wanted to make sure the earlier removal of that one pattern from `INSTALL_MYSQL_TEST`s `EXCL_GALERA` was still correct.

            cross Chris Ross added a comment - Okay. Just wanted to make sure the earlier removal of that one pattern from `INSTALL_MYSQL_TEST`s `EXCL_GALERA` was still correct.
            danblack Daniel Black added a comment -

            Yes. Resulting wsrep files (including both PRs) is per https://github.com/MariaDB/server/pull/2417#issue-1521525371. Two or three files might be excess to absolute minimal requirements but its better than going too far.

            danblack Daniel Black added a comment - Yes. Resulting wsrep files (including both PRs) is per https://github.com/MariaDB/server/pull/2417#issue-1521525371 . Two or three files might be excess to absolute minimal requirements but its better than going too far.

            People

              danblack Daniel Black
              robinnew Robin Newhouse
              Votes:
              0 Vote for this issue
              Watchers:
              4 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.