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

NUMA does not get enabled even when checks are passed

Details

    Description

      MDEV-10829 introduced a cmake check for NUMA libraries, which is presumably supposed to enable the logic if the checks pass. It does not seem to happen.

      -- Looking for include file numa.h
      -- Looking for include file numa.h - found
      -- Looking for include file numaif.h
      -- Looking for include file numaif.h - found
      -- Performing Test HAVE_LIBNUMA
      -- Performing Test HAVE_LIBNUMA - Success
      

      $ cmake . -LAH | grep -i numa
      // Explicitly set NUMA memory allocation policy
      WITH_NUMA:BOOL=ON
      

      $ grep NUMA include/my_config.h 
      #define HAVE_LIBNUMA 1
      

      But I still don't get innodb_numa_interleave variable, apparently because WITH_NUMA which is checked by ha_innodb.cc is not passed through any defines.

      Attachments

        Issue Links

          Activity

            wlad Vladislav Vaintroub added a comment - - edited

            Dan, why are the options and compile checks that only affect innodb are in config.h? Engines usually have nothing to look for there.
            Also, is the reason for libnuma only to workaround Linux memory management "swappiness" for huge allocations that spawn more that node0? Then it should only be checked on Linux with IF(CMAKE_SYSTEM_NAME MATCHES "Linux"). If this is valid for other Unixes, then
            IF(UNIX) is appropriate. There is no reason for irrelevant checks to be run on OSes that either have no libnuma, or do not expose the problem that innodb_numa_interleave tries to workaround.

            wlad Vladislav Vaintroub added a comment - - edited Dan, why are the options and compile checks that only affect innodb are in config.h? Engines usually have nothing to look for there. Also, is the reason for libnuma only to workaround Linux memory management "swappiness" for huge allocations that spawn more that node0? Then it should only be checked on Linux with IF(CMAKE_SYSTEM_NAME MATCHES "Linux"). If this is valid for other Unixes, then IF(UNIX) is appropriate. There is no reason for irrelevant checks to be run on OSes that either have no libnuma, or do not expose the problem that innodb_numa_interleave tries to workaround.
            danblack Daniel Black added a comment -

            Thanks elenst and wlad. I see JIRA is picking up pull requests - nice! I hope I've address all issues in the PR referenced.

            The innodb_numa_interleave is to give an average consistent response time from the memory level more than a high/low performance response time. Linux allocates all on one node before overflowing to the next. As such sql threads on CPUs associated with other nodes will incur a penalty to cross to the dominate buffer pool node to obtain data. I don't know how other OSs allocate memory however I suspect it might be similar.

            Getting libnuma into the dependencies was goal also to enable a better implementation colocation of sql threads and bufferpool on a node. Happy to work on this in a cross platform way. JIRA task coming when-time-permits(tm).

            danblack Daniel Black added a comment - Thanks elenst and wlad . I see JIRA is picking up pull requests - nice! I hope I've address all issues in the PR referenced. The innodb_numa_interleave is to give an average consistent response time from the memory level more than a high/low performance response time. Linux allocates all on one node before overflowing to the next. As such sql threads on CPUs associated with other nodes will incur a penalty to cross to the dominate buffer pool node to obtain data. I don't know how other OSs allocate memory however I suspect it might be similar. Getting libnuma into the dependencies was goal also to enable a better implementation colocation of sql threads and bufferpool on a node. Happy to work on this in a cross platform way. JIRA task coming when-time-permits(tm).
            elenst Elena Stepanova added a comment - - edited

            I don't quite understand from the comment above whether anything has been fixed, but I'm still not getting the variable in the builds on my machine. I'm also concerned that we don't have the variable on the very same machine where 5.7 would have it, it makes the migration more difficult.

            Maybe marko could take a look, i'm not sure things are being done correctly here.

            I don't really like re-opening an issue which was closed in a previous release, but I'm doing it instead of opening a new report because I'm not sure anything will be fixed here. If there happens to be a fix, depending on how big it is, please either commit it with a comment like "post fix for MDEV-11195 ...", or create a new report and re-close this one.

            elenst Elena Stepanova added a comment - - edited I don't quite understand from the comment above whether anything has been fixed, but I'm still not getting the variable in the builds on my machine. I'm also concerned that we don't have the variable on the very same machine where 5.7 would have it, it makes the migration more difficult. Maybe marko could take a look, i'm not sure things are being done correctly here. I don't really like re-opening an issue which was closed in a previous release, but I'm doing it instead of opening a new report because I'm not sure anything will be fixed here. If there happens to be a fix, depending on how big it is, please either commit it with a comment like "post fix for MDEV-11195 ...", or create a new report and re-close this one.

            In MySQL 5.7, it appears to me that the only effect of cmake -DWITH_NUMA=ON is that the build will be aborted if the NUMA library is not found. The C preprocessor symbol HAVE_LIBNUMA will be used for enabling the feature:

            IF(WITH_NUMA AND NOT HAVE_LIBNUMA)
              # Forget it in cache, abort the build.
              UNSET(WITH_NUMA CACHE)
              MESSAGE(FATAL_ERROR "NUMA library missing or required version not available")
            ENDIF()
             
            IF(HAVE_LIBNUMA AND NOT WITH_NUMA)
               SET(HAVE_LIBNUMA 0)
               MESSAGE(STATUS "Disabling NUMA on user's request")
            ENDIF()
            

            In MariaDB 10.2, cmake/numa.cmake is implementing similar logic, defining the preprocessor symbol HAVE_LIBNUMA if and only if NUMA is available and -DWITH_NUMA=OFF has not been specified. Just like MySQL 5.7, WITH_NUMA should never be defined as a C preprocessor symbol. The problem appears to be that in MariaDB 10.2, InnoDB is incorrectly checking for both symbols:

            #if defined(HAVE_LIBNUMA) && defined(WITH_NUMA)
            

            marko Marko Mäkelä added a comment - In MySQL 5.7, it appears to me that the only effect of cmake -DWITH_NUMA=ON is that the build will be aborted if the NUMA library is not found. The C preprocessor symbol HAVE_LIBNUMA will be used for enabling the feature: IF(WITH_NUMA AND NOT HAVE_LIBNUMA) # Forget it in cache, abort the build. UNSET(WITH_NUMA CACHE) MESSAGE(FATAL_ERROR "NUMA library missing or required version not available") ENDIF()   IF(HAVE_LIBNUMA AND NOT WITH_NUMA) SET(HAVE_LIBNUMA 0) MESSAGE(STATUS "Disabling NUMA on user's request") ENDIF() In MariaDB 10.2, cmake/numa.cmake is implementing similar logic, defining the preprocessor symbol HAVE_LIBNUMA if and only if NUMA is available and -DWITH_NUMA=OFF has not been specified. Just like MySQL 5.7, WITH_NUMA should never be defined as a C preprocessor symbol. The problem appears to be that in MariaDB 10.2, InnoDB is incorrectly checking for both symbols: #if defined(HAVE_LIBNUMA) && defined(WITH_NUMA)

            ok to push, good that code is simplified.

            jplindst Jan Lindström (Inactive) added a comment - ok to push, good that code is simplified.
            danblack Daniel Black added a comment -

            thanks marko

            danblack Daniel Black added a comment - thanks marko

            People

              marko Marko Mäkelä
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              5 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.