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

innodb_compression_algorithm configuration variable can be set to unsupported value

Details

    Description

      We should check is LZ4 and/or LZO installed and if not do not allow to use respective values on configuration variable. Similarly innodb_compression_algorithm_basic.test should be fixed to work when LZ4 and/or LZO is not installed.

      Attachments

        Issue Links

          Activity

            Few considerations:

            • from a certain point of view, sys_vars.innodb_compression_algorithm_basic should work independently from whether build system had lz4 and/or lzo.
            • on the other hand, we, probably, want our packages to have all features, and compile them with lz4 and lzo. So it'd be good if the test (this or another one) would fail on our packages that don't have lzo/lz4. This can be done by examining version_comment status variable.
            • on the yet another hand, we, probably, don't want to add run-time dependencies on lz4 and lzo for the server package, especially as 10.0 is GA and as the overwhelming majority of our users will never use this innodb compression (it's only useful for a specific filesystem on a specific hardware).
            serg Sergei Golubchik added a comment - Few considerations: from a certain point of view, sys_vars.innodb_compression_algorithm_basic should work independently from whether build system had lz4 and/or lzo. on the other hand, we, probably, want our packages to have all features, and compile them with lz4 and lzo. So it'd be good if the test (this or another one) would fail on our packages that don't have lzo/lz4. This can be done by examining version_comment status variable. on the yet another hand, we, probably, don't want to add run-time dependencies on lz4 and lzo for the server package, especially as 10.0 is GA and as the overwhelming majority of our users will never use this innodb compression (it's only useful for a specific filesystem on a specific hardware).

            Suggestion:

            • Add a new InnoDB status variables innodb_have_lz4 and innodb_have_lzo so that they would be true iff binary is compiled on system where library is present.
            • Modify storage/xtradb/handler/ha_innodb.cc and storage/innobase/handler/ha_innodb.cc, so that dynamic configuration varible innodb_compression_algorithm can be set only to those values that are supported by current system.
            • Modify innodb_compression_algorithm_basic.test to test all those features that are present based on InnoDB status variables above.
            jplindst Jan Lindström (Inactive) added a comment - Suggestion: Add a new InnoDB status variables innodb_have_lz4 and innodb_have_lzo so that they would be true iff binary is compiled on system where library is present. Modify storage/xtradb/handler/ha_innodb.cc and storage/innobase/handler/ha_innodb.cc, so that dynamic configuration varible innodb_compression_algorithm can be set only to those values that are supported by current system. Modify innodb_compression_algorithm_basic.test to test all those features that are present based on InnoDB status variables above.

            It doesn't solve the issues I've mentioned:

            • if we decide to build our binaries with lz4 and lzo, we want tests to fail in buildbot when lz4 and lzo are not available. But tests should not fail when not building a package.
            • it's unclear whether we want to build our binaries with lz4 and lzo at all — it's a new dependency, for GA binaries, and a useless dependency too (useless for most of users)
            serg Sergei Golubchik added a comment - It doesn't solve the issues I've mentioned: if we decide to build our binaries with lz4 and lzo, we want tests to fail in buildbot when lz4 and lzo are not available. But tests should not fail when not building a package. it's unclear whether we want to build our binaries with lz4 and lzo at all — it's a new dependency, for GA binaries, and a useless dependency too (useless for most of users)

            Is there a way to distinguish when we are running tests on buildbot and when running tests when not building a package ? Some external e.g. export BUILDBOT_BUILD_BINARIES=Y ?

            In my opinion, we should not build our binaries with lz4 or lzo, currently it is not clear if page compression is usefull on HDs. It is clear now that it is not that usefull if you have only one HD. Situation is not clear if you have RAID containing more than one HD.

            jplindst Jan Lindström (Inactive) added a comment - Is there a way to distinguish when we are running tests on buildbot and when running tests when not building a package ? Some external e.g. export BUILDBOT_BUILD_BINARIES=Y ? In my opinion, we should not build our binaries with lz4 or lzo, currently it is not clear if page compression is usefull on HDs. It is clear now that it is not that usefull if you have only one HD. Situation is not clear if you have RAID containing more than one HD.

            We can have "BUILDBOT_BUILD_BINARIES", it is possible. But above I suggested to check the version comment. Like in

            SELECT @@version_comment = 'mariadb.org binary distribution';

            This will match for packages that we build in buildbot. And this will match for someone, who has download our package and runs the test suite (which is correct, because our packages, even if downloaded, should have the same set of features). And this will not match in buildbot for builders that don't build packages (which is also correct, as they don't have to have the "release" configuration with all features). So, this is probably better than BUILDBOT_BUILD_BINARIES.

            But anyway, back to your second comment. As you're saying that we should not build with lz4 and lzo now, we don't need to run a separate set of tests in buildbot just now. So for now, current innodb_compression_algorithm_basic.test is good, it only checks for zlib and none values.

            And while the original bug stays — innodb_compression_algorithm configuration variable can still be set to unsupported value — all related issues (how to build release packages, extra dependencies, how to fix the test for buildbot), all these related issues seem to be solved Thanks!

            serg Sergei Golubchik added a comment - We can have "BUILDBOT_BUILD_BINARIES", it is possible. But above I suggested to check the version comment. Like in SELECT @@version_comment = 'mariadb.org binary distribution' ; This will match for packages that we build in buildbot. And this will match for someone, who has download our package and runs the test suite (which is correct, because our packages, even if downloaded, should have the same set of features). And this will not match in buildbot for builders that don't build packages (which is also correct, as they don't have to have the "release" configuration with all features). So, this is probably better than BUILDBOT_BUILD_BINARIES. But anyway, back to your second comment. As you're saying that we should not build with lz4 and lzo now, we don't need to run a separate set of tests in buildbot just now. So for now, current innodb_compression_algorithm_basic.test is good, it only checks for zlib and none values. And while the original bug stays — innodb_compression_algorithm configuration variable can still be set to unsupported value — all related issues (how to build release packages, extra dependencies, how to fix the test for buildbot), all these related issues seem to be solved Thanks!

            commit 3bca01930aa5298cf6a8d602e66ddcea054edf50
            Author: Jan Lindström <jan.lindstrom@skysql.com>
            Date: Wed Jun 25 08:46:54 2014 +0300

            MDEV-6361: innodb_compression_algorithm configuration variable can
            be set to unsupported value.

            MDEV-6350: Excessive unnecessary memory allocation at InnoDB/XtraDB
            startup if LZO is installed.

            jplindst Jan Lindström (Inactive) added a comment - commit 3bca01930aa5298cf6a8d602e66ddcea054edf50 Author: Jan Lindström <jan.lindstrom@skysql.com> Date: Wed Jun 25 08:46:54 2014 +0300 MDEV-6361 : innodb_compression_algorithm configuration variable can be set to unsupported value. MDEV-6350 : Excessive unnecessary memory allocation at InnoDB/XtraDB startup if LZO is installed.

            10.0-FusionIO

            revno: 3988
            committer: Jan Lindström <jplindst@mariadb.org>
            branch nick: 10.0-FusionIO-release
            timestamp: Thu 2014-06-26 07:50:48 +0300
            message:
            MDEV-6361: innodb_compression_algorithm configuration variable can
            be set to unsupported value.

            MDEV-6350: Excessive unnecessary memory allocation at InnoDB/XtraDB
            startup if LZO is installed.

            MDEV-6376: InnoDB: Assertion failure in thread 139995225970432
            in file buf0mtflu.cc line 570.

            jplindst Jan Lindström (Inactive) added a comment - 10.0-FusionIO revno: 3988 committer: Jan Lindström <jplindst@mariadb.org> branch nick: 10.0-FusionIO-release timestamp: Thu 2014-06-26 07:50:48 +0300 message: MDEV-6361 : innodb_compression_algorithm configuration variable can be set to unsupported value. MDEV-6350 : Excessive unnecessary memory allocation at InnoDB/XtraDB startup if LZO is installed. MDEV-6376 : InnoDB: Assertion failure in thread 139995225970432 in file buf0mtflu.cc line 570.

            People

              jplindst Jan Lindström (Inactive)
              jplindst Jan Lindström (Inactive)
              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.