Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YARN-11743. Cgroup v2 support should fall back to v1 when there are no v2 controllers #7222

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ public class ResourceHandlerModule {
private static volatile CpuResourceHandler
cGroupsCpuResourceHandler;

private static void initializeCGroupHandlers(Configuration conf)
throws ResourceHandlerException {
private static void initializeCGroupHandlers(Configuration conf,
CGroupsHandler.CGroupController controller) throws ResourceHandlerException {
initializeCGroupV1Handler(conf);
if (cgroupsV2Enabled) {
if (cgroupsV2Enabled && !isMountedInCGroupsV1(controller)) {
initializeCGroupV2Handler(conf);
Comment on lines 81 to 83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: do we need to initialize the v1 handler if we're looking at clean v2 case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @brumi1024! With the current logic we use the v1 handler to get the v1 controller paths and init v2 handler only if the given controller is not mounted in v1. But with a pure v2 setup I don't think we will use v1 handler after that check, there may be a cleaner way to determine this.

It's not the clean v2 setup, but with the resource plugins we still use only the v1 handler in this codepart, so probably additional checks would be needed there as well if we decide not to init the v1 handler.

}
}
Expand Down Expand Up @@ -110,6 +110,10 @@ private static void initializeCGroupV2Handler(Configuration conf)
}
}

private static boolean isMountedInCGroupsV1(CGroupsHandler.CGroupController controller) {
return (cGroupV1Handler != null && cGroupV1Handler.getControllerPath(controller) != null);
}

private static boolean isMountedInCGroupsV2(CGroupsHandler.CGroupController controller) {
return (cGroupV2Handler != null && cGroupV2Handler.getControllerPath(controller) != null);
}
Expand Down Expand Up @@ -174,7 +178,7 @@ private static CpuResourceHandler initCGroupsCpuResourceHandler(
if (cGroupsCpuResourceHandler == null) {
LOG.debug("Creating new cgroups cpu handler");

initializeCGroupHandlers(conf);
initializeCGroupHandlers(conf, CGroupsHandler.CGroupController.CPU);
if (isMountedInCGroupsV2(CGroupsHandler.CGroupController.CPU)) {
cGroupsCpuResourceHandler = new CGroupsV2CpuResourceHandlerImpl(cGroupV2Handler);
} else {
Expand All @@ -198,7 +202,7 @@ private static CpuResourceHandler initCGroupsCpuResourceHandler(
if (trafficControlBandwidthHandler == null) {
LOG.info("Creating new traffic control bandwidth handler.");

initializeCGroupHandlers(conf);
initializeCGroupHandlers(conf, CGroupsHandler.CGroupController.NET_CLS);
trafficControlBandwidthHandler = new
TrafficControlBandwidthHandlerImpl(PrivilegedOperationExecutor
.getInstance(conf), cGroupV1Handler,
Expand Down Expand Up @@ -235,7 +239,7 @@ public static ResourceHandler getNetworkTaggingHandler(Configuration conf)
if (networkPacketTaggingHandlerImpl == null) {
LOG.info("Creating new network-tagging-handler.");

initializeCGroupHandlers(conf);
initializeCGroupHandlers(conf, CGroupsHandler.CGroupController.NET_CLS);
networkPacketTaggingHandlerImpl =
new NetworkPacketTaggingHandlerImpl(
PrivilegedOperationExecutor.getInstance(conf), cGroupV1Handler);
Expand Down Expand Up @@ -267,7 +271,7 @@ private static CGroupsBlkioResourceHandlerImpl getCgroupsBlkioResourceHandler(
if (cGroupsBlkioResourceHandler == null) {
LOG.debug("Creating new cgroups blkio handler");

initializeCGroupHandlers(conf);
initializeCGroupHandlers(conf, CGroupsHandler.CGroupController.BLKIO);
cGroupsBlkioResourceHandler =
new CGroupsBlkioResourceHandlerImpl(cGroupV1Handler);
}
Expand All @@ -292,7 +296,7 @@ public static MemoryResourceHandler initMemoryResourceHandler(
synchronized (MemoryResourceHandler.class) {
if (cGroupsMemoryResourceHandler == null) {

initializeCGroupHandlers(conf);
initializeCGroupHandlers(conf, CGroupsHandler.CGroupController.MEMORY);
if (isMountedInCGroupsV2(CGroupsHandler.CGroupController.MEMORY)) {
cGroupsMemoryResourceHandler = new CGroupsV2MemoryResourceHandlerImpl(cGroupV2Handler);
} else {
Expand Down Expand Up @@ -359,7 +363,7 @@ private static void addHandlersFromConfiguredResourcePlugins(
}

for (ResourcePlugin plugin : pluginMap.values()) {
initializeCGroupHandlers(conf);
initializeCGroupV1Handler(conf);
addHandlerIfNotNull(handlerList,
plugin.createResourceHandler(nmContext,
cGroupV1Handler,
Expand Down
Loading