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

MTR Safe Process Hardcodes RLIMIT_NOFILE

Details

    Description

      The mariadbd instances that are spawned by MTR have their RLIMIT_NOFILE hardcoded to 1024. This makes MTR unusable if used for high concurrency testing (e.g. for benchmarking).

      The hardcoding is set in ./mysql-test/lib/My/SafeProcess/safe_process.cc in its main function:

          /*
            mysqld defaults depend on that. make test results stable and independent
            from the environment
          */
          setlimit(RLIMIT_NOFILE, 1024, 1024);
      

      Instead, the safe process executable should use the test's configuration and use its value for open_files_limit.

      Attachments

        Activity

          Jack Lyu Jack Lyu added a comment -

          Hi all, this is my first time trying to do some contribution to this community. I saw this task is labelled "beginner-friendly" so that I wanna give it a try.

          Here's what I am thinking about the fix :

          Currently to run this program, user doesn't need to provide a soft/hard limit in the command line, because the soft/hard limit is hardcoded to 1024. To fix the issue, we need to give the user an option to pass in two additional arguments (i.e. soft/hard limit) when they invoke the program, then get those 2 arguments in the /parse arguments/ section, then pass them to the setlimit() function. If the user didn't provide soft/hard limit, just default them to 1024.

          Is my understand correct? If this is confirmed I'll start working on it when I get up. Thank you!

          Jack Lyu Jack Lyu added a comment - Hi all, this is my first time trying to do some contribution to this community. I saw this task is labelled "beginner-friendly" so that I wanna give it a try. Here's what I am thinking about the fix : Currently to run this program, user doesn't need to provide a soft/hard limit in the command line, because the soft/hard limit is hardcoded to 1024. To fix the issue, we need to give the user an option to pass in two additional arguments (i.e. soft/hard limit) when they invoke the program, then get those 2 arguments in the / parse arguments / section, then pass them to the setlimit() function. If the user didn't provide soft/hard limit, just default them to 1024. Is my understand correct? If this is confirmed I'll start working on it when I get up. Thank you!
          Roel Roel Van de Paar added a comment - - edited

          Jack Lyu Thank you for being a first-time contributor! Much appreciated.

          I had been thinking about what would be a good fix for this issue, and something did not immediately come to mind. The issue is that on one side we want to keep the default (1024), for one so there won't be runaway jobs in testing setups - in case a testcase has a bug for example, but on the other hand we want to be able to increase this value easily when needed.

          Your solution seems to enable both: passing an argument (a single one may do to set both?) - leaving the default 1024 yet enabling the user to easily change the RLIMIT_NOFILE limit. We may also at the same time add some smarts like automatically/intelligibly setting --max-connections= (MTR max connections), --mysqld=--max-connections= (server max connections), and --mysqld=--open-files-limit= (open files limit for the server) when the new option is used.

          serg elenst fyi

          Roel Roel Van de Paar added a comment - - edited Jack Lyu Thank you for being a first-time contributor! Much appreciated. I had been thinking about what would be a good fix for this issue, and something did not immediately come to mind. The issue is that on one side we want to keep the default (1024), for one so there won't be runaway jobs in testing setups - in case a testcase has a bug for example, but on the other hand we want to be able to increase this value easily when needed. Your solution seems to enable both: passing an argument (a single one may do to set both?) - leaving the default 1024 yet enabling the user to easily change the RLIMIT_NOFILE limit. We may also at the same time add some smarts like automatically/intelligibly setting --max-connections= (MTR max connections), --mysqld=--max-connections= (server max connections), and --mysqld=--open-files-limit= (open files limit for the server) when the new option is used. serg elenst fyi
          Jack Lyu Jack Lyu added a comment - - edited

          Thank you so much for your response!

          Currently the [options to safe_process] includes only "verbose", "parent-pid=", "nocore", and "env", what I need to do is to extend this list and adds the "open-files-limit=" case (while keep the default 1024 if the user doesn't provide this limit), is that correct?

          Also at the start of the file it says

          $> safe_process --output=output.log – mysqld --datadir=var/data1 ... will redirect output to output.log.

          However, "--output=" is not an option to safe_process (since it is not a case in the parse arguments section starting at line 243 in the main function). Is it handled somewhere else? Or is this a mismatch?

          Thank you in advance!

          Jack Lyu Jack Lyu added a comment - - edited Thank you so much for your response! Currently the [options to safe_process] includes only "verbose", "parent-pid=", "nocore", and "env", what I need to do is to extend this list and adds the "open-files-limit=" case (while keep the default 1024 if the user doesn't provide this limit), is that correct? Also at the start of the file it says $> safe_process --output=output.log – mysqld --datadir=var/data1 ... will redirect output to output.log. However, "--output=" is not an option to safe_process (since it is not a case in the parse arguments section starting at line 243 in the main function). Is it handled somewhere else? Or is this a mismatch? Thank you in advance!

          Hi Jack Lyu! Thanks for looking into this!

          I would say to follow the pattern established by the nocore option, i.e. mysql-test/mariadb-test-run.pl reads it as a part of extra arguments (which are set my mtr parameters as either "--mysqld=" or in a .opt file), and it is then passed as an argument to the safe_process executable. The argument that we'd want to check in mariadb-test-run.pl would be --open-files-limit (which already exists). It should also be fine to use that value for both the soft and hard limit.

          Indeed it looks like the --output option is not used directly by safe_process.cc, but it is handled by perl before calling it. See the function create_process in mysql-test/lib/My/SafeProcess/Base.pm. If you could update that comment at the top of safe_process.cc to be consistent with its actual behavior, that would be greatly appreciated!

          bnestere Brandon Nesterenko added a comment - Hi Jack Lyu ! Thanks for looking into this! I would say to follow the pattern established by the nocore option, i.e. mysql-test/mariadb-test-run.pl reads it as a part of extra arguments (which are set my mtr parameters as either "--mysqld=" or in a .opt file), and it is then passed as an argument to the safe_process executable. The argument that we'd want to check in mariadb-test-run.pl would be --open-files-limit (which already exists). It should also be fine to use that value for both the soft and hard limit. Indeed it looks like the --output option is not used directly by safe_process.cc, but it is handled by perl before calling it. See the function create_process in mysql-test/lib/My/SafeProcess/Base.pm. If you could update that comment at the top of safe_process.cc to be consistent with its actual behavior, that would be greatly appreciated!

          you don't really need to pass an argument to safe_process. you can run ulimit from perl, between the fork and exec

          serg Sergei Golubchik added a comment - you don't really need to pass an argument to safe_process. you can run ulimit from perl, between the fork and exec
          Jack Lyu Jack Lyu added a comment -

          @Brandon and Sergei. Thank you so much for your help! I spent the whole afternoon reading the source code and documentation on Perl and finally see what you mean. "the pattern established by the nocore option" indeed is a really good hint. Will get it done asap!

          Jack Lyu Jack Lyu added a comment - @Brandon and Sergei. Thank you so much for your help! I spent the whole afternoon reading the source code and documentation on Perl and finally see what you mean. "the pattern established by the nocore option" indeed is a really good hint. Will get it done asap!

          People

            Unassigned Unassigned
            bnestere Brandon Nesterenko
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:

              Git Integration

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