Change how implementations of PeriodicMemoryChecker are selected #24944
Replies: 2 comments
-
@spershin @tangjiangling We can discuss this in the next native worker group meeting. |
Beta Was this translation helpful? Give feedback.
-
@czentgr What we do internally at Meta is simply have a class that inherits from That's it. The default This is done not to break current use cases. Ideally, it would be good for any implementation of presto_cpp to have that override if we need a special tracker and drop the macro altogether. Let me know if this made the issue more clear. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
PR #24922 made a PeriodicMemoryChecker to be part of PrestoServer.
The LinuxMemoryChecker is an implementation that can be enabled by adding a build flag (macro) to select it
PRESTO_MEMORY_CHECKER_TYPE
:presto/presto-native-execution/presto_cpp/main/CMakeLists.txt
Line 144 in b2919fa
In the PR it was discussed that perhaps this isn't the best way going forward and a discussion with the OSS community needs to occur. See #24922 (comment)
Meta also has its own implementation of the PeriodicMemoryChecker used internally: #24922 (comment)
We use the LinuxMemoryChecker in our own builds but are open to changes in how it can be selected. The main reason for doing it the way it is done is that users of PrestoC++ have a choice at build time which code of MemoryChecker to use. A user would drop in their own C++ file and add to the CMakeLists.txt and as a result select to build that file into the executable. The usage would then be configured through the config.properties.
@spershin @tangjiangling @majetideepak @minhancao
Beta Was this translation helpful? Give feedback.
All reactions