Add LinuxMemoryChecker checks/warnings to ensure system-memory-gb < s…

…ystem-mem-limit-gb < actual total memory capacity.

presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp

@@ -36,6 +36,19 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
       statFile_ = "None";
     }
     LOG(INFO) << fmt::format("Using memory stat file {}", statFile_);
+
+    // Set memMaxFile_ to:
+    // "/sys/fs/cgroup/memory/memory.limit_in_bytes" for cgroup v1
+    // or
+    // "/sys/fs/cgroup/memory.max" for cgroup v2.
+    if ((stat(kCgroupV1MaxMemFilePath, &buffer) == 0)) {
+      memMaxFile_ = kCgroupV1MaxMemFilePath;
+    } else if ((stat(kCgroupV2MaxMemFilePath, &buffer) == 0)) {
+      memMaxFile_ = kCgroupV2MaxMemFilePath;
+    } else {
+      memMaxFile_ = "None";
+    }
+    LOG(INFO) << fmt::format("Using memory max file {}", memMaxFile_);
   }
 
   ~LinuxMemoryChecker() override {}
@@ -49,6 +62,73 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
     LOG(INFO) << fmt::format("Changed to using memory stat file {}", statFile_);
   }
 
+  void setMemMaxFile(std::string memMaxFile) {
+    memMaxFile_ = memMaxFile;
+    LOG(INFO) << fmt::format(
+        "Changed to using memory max file {}", memMaxFile_);
+  }
+
+  void setMemInfo(std::string memInfoFile) {
+    memInfoFile_ = memInfoFile;
+    LOG(INFO) << fmt::format("Changed to using meminfo file {}", memInfoFile_);
+  }
+
+  void start() {
+    // Check system-memory-gb < system-mem-limit-gb < actual total memory
+    // capacity.
+    VELOX_CHECK_LE(
+        config_.systemMemLimitBytes,
+        getActualTotalMemory(),
+        "system-mem-limit-gb is higher than the actual total memory capacity.");
+
+    auto* systemConfig = SystemConfig::instance();
+    if (config_.systemMemLimitBytes < systemConfig->systemMemoryGb()) {
+      LOG(WARNING) << "system-mem-limit-gb is smaller than system-memory-gb. "
+                   << "Expected: system-mem-limit-gb >= system-memory-gb.";
+    }
+
+    PeriodicMemoryChecker::start();
+  }
+
+  int64_t getActualTotalMemory() {
+    // Set actual total memory to be the smaller number between /proc/meminfo
+    // and memMaxFile_.
+    int64_t actualTotalMemory = 0;
+    folly::gen::byLine(memInfoFile_.c_str()) |
+        [&](const folly::StringPiece& line) -> void {
+      if (actualTotalMemory != 0) {
+        return;
+      }
+      actualTotalMemory = static_cast<int64_t>(
+          extractNumericConfigValueWithRegex(line, kMemTotalRegex) * 1024);
+    };
+
+    // For cgroup v1, memory.limit_in_bytes can default to a really big numeric
+    // value in bytes like 9223372036854771712 to represent that
+    // memory.limit_in_bytes is not set to a value. The default value here is
+    // set to PAGE_COUNTER_MAX, which is LONG_MAX/PAGE_SIZE on 64-bit platform.
+    // The default value can vary based upon the platform's PAGE_SIZE. If
+    // memory.limit_in_bytes contains a really big numeric value, then we will
+    // use MemTotal from /proc/meminfo.
+
+    // For cgroup v2, memory.max can contain a numeric value in bytes or string
+    // "max" which represents no value has been set. If memory.max contains
+    // "max", then we will use MemTotal from /proc/meminfo.
+    if (memMaxFile_ != "None") {
+      folly::gen::byLine(memMaxFile_.c_str()) |
+          [&](const folly::StringPiece& line) -> void {
+        if (line == "max") {
+          return;
+        }
+        actualTotalMemory =
+            std::min(actualTotalMemory, folly::to<int64_t>(line));
+        return;
+      };
+    }
+
+    return actualTotalMemory;
+  }
+
  protected:
   // Current memory calculation used is inactive_anon + active_anon
   // Our first attempt was using memInfo memTotal - memAvailable.
@@ -83,6 +163,10 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
     if (statFile_ != "None") {
       folly::gen::byLine(statFile_.c_str()) |
           [&](const folly::StringPiece& line) -> void {
+        if (activeAnon != 0 && inactiveAnon != 0) {
+          return;
+        }
+
         if (inactiveAnon == 0) {
           inactiveAnon =
               extractNumericConfigValueWithRegex(line, kInactiveAnonRegex);
@@ -92,19 +176,19 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
           activeAnon =
               extractNumericConfigValueWithRegex(line, kActiveAnonRegex);
         }
-
-        if (activeAnon != 0 && inactiveAnon != 0) {
-          return;
-        }
       };
 
       // Unit is in bytes.
       return inactiveAnon + activeAnon;
     }
 
     // Last resort use host machine info.
-    folly::gen::byLine("/proc/meminfo") |
+    folly::gen::byLine(memInfoFile_.c_str()) |
         [&](const folly::StringPiece& line) -> void {
+      if (memAvailable != 0 && memTotal != 0) {
+        return;
+      }
+
       if (memAvailable == 0) {
         memAvailable =
             extractNumericConfigValueWithRegex(line, kMemAvailableRegex) * 1024;
@@ -114,10 +198,6 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
         memTotal =
             extractNumericConfigValueWithRegex(line, kMemTotalRegex) * 1024;
       }
-
-      if (memAvailable != 0 && memTotal != 0) {
-        return;
-      }
     };
     // Unit is in bytes.
     return (memAvailable && memTotal) ? memTotal - memAvailable : 0;
@@ -143,10 +223,15 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
   const boost::regex kInactiveAnonRegex{R"!(inactive_anon\s*(\d+)\s*)!"};
   const boost::regex kActiveAnonRegex{R"!(active_anon\s*(\d+)\s*)!"};
   const boost::regex kMemAvailableRegex{R"!(MemAvailable:\s*(\d+)\s*kB)!"};
-  const boost::regex kMemTotalRegex{R"!(MemTotal:\s*(\d+)\s*kB)!"};
+  const boost::regex kMemTotalRegex{R"!(MemTotal:\s*(\d+)\s+kB)!"};
   const char* kCgroupV1Path = "/sys/fs/cgroup/memory/memory.stat";
   const char* kCgroupV2Path = "/sys/fs/cgroup/memory.stat";
+  const char* kCgroupV1MaxMemFilePath =
+      "/sys/fs/cgroup/memory/memory.limit_in_bytes";
+  const char* kCgroupV2MaxMemFilePath = "/sys/fs/cgroup/memory.max";
   std::string statFile_;
+  std::string memInfoFile_ = "/proc/meminfo";
+  std::string memMaxFile_;
 
   size_t extractNumericConfigValueWithRegex(
       const folly::StringPiece& line,

presto-native-execution/presto_cpp/main/tests/LinuxMemoryCheckerTest.cpp

@@ -21,24 +21,34 @@
 namespace fs = boost::filesystem;
 
 namespace {
-std::string getStatsFilePath(const std::string& fileName) {
+std::string getFilePath(const std::string& fileName) {
   return fs::current_path().string() + "/examples/" + fileName;
 }
 } // namespace
 
 namespace facebook::presto {
 class LinuxMemoryCheckerTest : public testing::Test {
  protected:
-  LinuxMemoryChecker memChecker;
+  LinuxMemoryChecker memChecker_;
 
-  LinuxMemoryCheckerTest() : memChecker(PeriodicMemoryChecker::Config{}) {}
+  LinuxMemoryCheckerTest() : memChecker_(PeriodicMemoryChecker::Config{}) {}
+
+  void checkMemoryMax(
+      const std::string& memMaxFileName,
+      int64_t expectedMemoryMax) {
+    auto memInfoFilePath = getFilePath("meminfo");
+    memChecker_.setMemInfo(memInfoFilePath);
+    auto memMaxFilePath = getFilePath(memMaxFileName);
+    memChecker_.setMemMaxFile(memMaxFilePath);
+    ASSERT_EQ(memChecker_.getActualTotalMemory(), expectedMemoryMax);
+  }
 
   void checkMemoryUsage(
       const std::string& statFileName,
       int64_t expectedMemoryUsage) {
-    auto statFilePath = getStatsFilePath(statFileName);
-    memChecker.setStatFile(statFilePath);
-    ASSERT_EQ(memChecker.getUsedMemory(), expectedMemoryUsage);
+    auto statFilePath = getFilePath(statFileName);
+    memChecker_.setStatFile(statFilePath);
+    ASSERT_EQ(memChecker_.getUsedMemory(), expectedMemoryUsage);
   }
 };
 
@@ -60,12 +70,95 @@ TEST_F(LinuxMemoryCheckerTest, basic) {
       LinuxMemoryChecker(PeriodicMemoryChecker::Config{
           1'000, true, 1024, 32, true, 5, "/path/to/dir", "", 5, 512}),
       "heapDumpFilePrefix cannot be empty when heap dump is enabled.");
-  LinuxMemoryChecker memChecker(PeriodicMemoryChecker::Config{
+  LinuxMemoryChecker memChecker_(PeriodicMemoryChecker::Config{
       1'000, false, 0, 0, false, 5, "/path/to/dir", "prefix", 5, 512});
 
+  ASSERT_NO_THROW(memChecker_.start());
+  VELOX_ASSERT_THROW(memChecker_.start(), "start() called more than once");
+  ASSERT_NO_THROW(memChecker_.stop());
+}
+
+TEST_F(LinuxMemoryCheckerTest, sysMemLimitBytesCheck) {
+  // system-mem-limit-gb should be set less than or equal to
+  // the actual total memory available.
+  auto memInfoFilePath = getFilePath("meminfo");
+  auto memMaxFilePath = getFilePath("memory131gb.max");
+
+  // systemMemLimitBytes = 130,000,000,000 bytes.
+  // actual total memory = 131,000,000,000 bytes.
+  LinuxMemoryChecker memChecker(PeriodicMemoryChecker::Config{
+      1'000,
+      true,
+      130000000000,
+      32,
+      true,
+      5,
+      "/path/to/dir",
+      "prefix",
+      5,
+      512});
+  memChecker.setMemInfo(memInfoFilePath);
+  memChecker.setMemMaxFile(memMaxFilePath);
   ASSERT_NO_THROW(memChecker.start());
-  VELOX_ASSERT_THROW(memChecker.start(), "start() called more than once");
   ASSERT_NO_THROW(memChecker.stop());
+
+  // systemMemLimitBytes = 131,000,000,001 bytes.
+  // actual total memory = 131,000,000,000 bytes.
+  LinuxMemoryChecker memChecker2(PeriodicMemoryChecker::Config{
+      1'000,
+      true,
+      131000000001,
+      32,
+      true,
+      5,
+      "/path/to/dir",
+      "prefix",
+      5,
+      512});
+  memChecker2.setMemInfo(memInfoFilePath);
+  memChecker2.setMemMaxFile(memMaxFilePath);
+  VELOX_ASSERT_THROW(memChecker2.start(), "(131000000001 vs. 131000000000)");
+  VELOX_ASSERT_THROW(memChecker2.stop(), "");
+}
+
+TEST_F(LinuxMemoryCheckerTest, memory131gbMax) {
+  // Testing for cgroup v1 and v2.
+  // memory131gb.max is 131,000,000,000 bytes.
+  // meminfo is 132,397,334,528 bytes.
+  // The expected actual memory should be 131,000,000,000 bytes here.
+  checkMemoryMax("memory131gb.max", 131000000000);
+}
+
+TEST_F(LinuxMemoryCheckerTest, memory133gbMax) {
+  // Testing for cgroup v1 and v2.
+  // memory133gb.max is 133,000,000,000 bytes.
+  // meminfo is 132,397,334,528 bytes.
+  // The expected actual memory should be 132,397,334,528 bytes here.
+  checkMemoryMax("memory133gb.max", 132397334528);
+}
+
+TEST_F(LinuxMemoryCheckerTest, cgroupV1MemoryMaxNotSet) {
+  // Testing for cgroup v1.
+  // When memory.limit_in_bytes is not set to a value, it could default to
+  // a huge value like 9223372036854771712 bytes.
+  // The default value is set to PAGE_COUNTER_MAX, which is LONG_MAX/PAGE_SIZE
+  // on 64-bit platform. The default value can vary based upon the platform's
+  // PAGE_SIZE.
+
+  // cgroupV1memoryNotSet.limit_in_bytes is 9,223,372,036,854,771,712 bytes.
+  // meminfo is 132,397,334,528 bytes.
+  // The expected actual memory should be 132,397,334,528 bytes here.
+  checkMemoryMax("cgroupV1memoryNotSet.limit_in_bytes", 132397334528);
+}
+
+TEST_F(LinuxMemoryCheckerTest, cgroupV2MemoryMaxNotSet) {
+  // Testing for cgroup v2.
+  // When memory.max is not set to a value, it defaults to contain string "max".
+
+  // cgroupV2memoryNotSet.max is "max".
+  // meminfo is 132,397,334,528 bytes.
+  // The expected actual memory should be 132,397,334,528 bytes here.
+  checkMemoryMax("cgroupV2memoryNotSet.max", 132397334528);
 }
 
 TEST_F(LinuxMemoryCheckerTest, memoryStatFileV1) {
@@ -81,7 +174,7 @@ TEST_F(LinuxMemoryCheckerTest, memoryStatFileV2) {
 TEST_F(LinuxMemoryCheckerTest, hostMachineInfo) {
   // Testing host machine info /proc/meminfo when None is specified for stat
   // file.
-  memChecker.setStatFile("None");
-  ASSERT_GT(memChecker.getUsedMemory(), 0);
+  memChecker_.setStatFile("None");
+  ASSERT_GT(memChecker_.getUsedMemory(), 0);
 }
 } // namespace facebook::presto

presto-native-execution/presto_cpp/main/tests/examples/cgroupV1memoryNotSet.limit_in_bytes

@@ -0,0 +1 @@
+9223372036854771712

presto-native-execution/presto_cpp/main/tests/examples/cgroupV2memoryNotSet.max

@@ -0,0 +1 @@
+max

presto-native-execution/presto_cpp/main/tests/examples/meminfo

@@ -0,0 +1,57 @@
+MemTotal:       129294272 kB
+MemFree:        127334232 kB
+MemAvailable:   127637400 kB
+Buffers:            2948 kB
+Cached:          1315676 kB
+SwapCached:            0 kB
+Active:           769056 kB
+Inactive:         810360 kB
+Active(anon):     277920 kB
+Inactive(anon):        0 kB
+Active(file):     491136 kB
+Inactive(file):   810360 kB
+Unevictable:          12 kB
+Mlocked:              12 kB
+SwapTotal:             0 kB
+SwapFree:              0 kB
+Zswap:                 0 kB
+Zswapped:              0 kB
+Dirty:                 4 kB
+Writeback:             0 kB
+AnonPages:        257672 kB
+Mapped:           341044 kB
+Shmem:             17128 kB
+KReclaimable:      70060 kB
+Slab:             172876 kB
+SReclaimable:      70060 kB
+SUnreclaim:       102816 kB
+KernelStack:        6640 kB
+PageTables:         5832 kB
+SecPageTables:         0 kB
+NFS_Unstable:          0 kB
+Bounce:                0 kB
+WritebackTmp:          0 kB
+CommitLimit:    64647136 kB
+Committed_AS:   1077692288 kB
+VmallocTotal:   34359738367 kB
+VmallocUsed:       22332 kB
+VmallocChunk:          0 kB
+Percpu:             7616 kB
+HardwareCorrupted:     0 kB
+AnonHugePages:    141312 kB
+ShmemHugePages:        0 kB
+ShmemPmdMapped:        0 kB
+FileHugePages:      2048 kB
+FilePmdMapped:         0 kB
+CmaTotal:              0 kB
+CmaFree:               0 kB
+Unaccepted:            0 kB
+HugePages_Total:       0
+HugePages_Free:        0
+HugePages_Rsvd:        0
+HugePages_Surp:        0
+Hugepagesize:       2048 kB
+Hugetlb:               0 kB
+DirectMap4k:      264100 kB
+DirectMap2M:     9156608 kB
+DirectMap1G:    122683392 kB

presto-native-execution/presto_cpp/main/tests/examples/memory131gb.max

@@ -0,0 +1 @@
+131000000000

presto-native-execution/presto_cpp/main/tests/examples/memory133gb.max

@@ -0,0 +1 @@
+133000000000