Skip to content

Commit

Permalink
Added tests and addressed PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@minhancao
minhancao committed Dec 3, 2024
1 parent 7646600 commit 4ae2cee
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 40 deletions.
93 changes: 55 additions & 38 deletions presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,39 +49,65 @@ 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() {
// Set memMaxFile to "/sys/fs/cgroup/memory/memory.limit_in_bytes" for
// cgroup v1 or "sys/fs/cgroup/memory.max" for cgroup v2.
// Set memMaxFile_ to:
// "/sys/fs/cgroup/memory/memory.limit_in_bytes" for cgroup v1
// or
// "/sys/fs/cgroup/memory.max" for cgroup v2.
struct stat buffer;
std::string memMaxFile;
if ((stat(kCgroupV1MaxMemFilePath, &buffer) == 0)) {
memMaxFile = kCgroupV1MaxMemFilePath;
memMaxFile_ = kCgroupV1MaxMemFilePath;
} else if ((stat(kCgroupV2MaxMemFilePath, &buffer) == 0)) {
memMaxFile = kCgroupV2MaxMemFilePath;
memMaxFile_ = kCgroupV2MaxMemFilePath;
} else {
memMaxFile = "None";
memMaxFile_ = "None";
}
LOG(INFO) << fmt::format("Checking {}", memMaxFile);
LOG(INFO) << fmt::format("Checking {}", memMaxFile_);

// 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.");

// For cgroup v1:
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 memory.limit_in_bytes
// and memMaxFile_.
int64_t actualTotalMemory = 0;
folly::gen::byLine("/proc/meminfo") |
folly::gen::byLine(memInfoFile_.c_str()) |
[&](const folly::StringPiece& line) -> void {
if (actualTotalMemory == 0) {
actualTotalMemory = static_cast<int64_t>(
extractNumericConfigValueWithRegex(line, kMemTotalRegex) * 1024);
}
if (actualTotalMemory != 0) {
return;
}
actualTotalMemory = static_cast<int64_t>(
extractNumericConfigValueWithRegex(line, kMemTotalRegex) * 1024);
};
if (memMaxFile != "None") {
folly::gen::byLine(memMaxFile.c_str()) |
// 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;
Expand All @@ -92,18 +118,7 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker {
};
}

VELOX_CHECK_LT(
config_.systemMemLimitBytes,
actualTotalMemory,
"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();
return actualTotalMemory;
}

protected:
Expand Down Expand Up @@ -140,6 +155,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);
Expand All @@ -149,19 +168,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;
Expand All @@ -171,10 +190,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;
Expand All @@ -200,13 +215,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,
Expand Down
54 changes: 52 additions & 2 deletions presto-native-execution/presto_cpp/main/tests/LinuxMemoryCheckerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
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
Expand All @@ -33,10 +33,20 @@ class LinuxMemoryCheckerTest : public testing::Test {

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);
auto statFilePath = getFilePath(statFileName);
memChecker.setStatFile(statFilePath);
ASSERT_EQ(memChecker.getUsedMemory(), expectedMemoryUsage);
}
Expand Down Expand Up @@ -68,6 +78,46 @@ TEST_F(LinuxMemoryCheckerTest, basic) {
ASSERT_NO_THROW(memChecker.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) {
// Testing cgroup v1 memory.stat file.
checkMemoryUsage("cgroupV1memory.stat", 5136384);
Expand Down
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/tests/examples/cgroupV1memoryNotSet.limit_in_bytes
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9223372036854771712
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/tests/examples/cgroupV2memoryNotSet.max
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
max
57 changes: 57 additions & 0 deletions presto-native-execution/presto_cpp/main/tests/examples/meminfo
View file
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/tests/examples/memory131gb.max
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
131000000000
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/tests/examples/memory133gb.max
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
133000000000

0 comments on commit 4ae2cee

Please sign in to comment.