[ https://issues.apache.org/jira/browse/YARN-5301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15949819#comment-15949819 ]
Daniel Templeton commented on YARN-5301: ---------------------------------------- Thanks, [~miklos.szeg...@cloudera.com]. A few comments: In {{parseMtab()}}, should {{cgroupList}} be a {{Set}} instead? On all the {{StringBuilder}} => string concatenation changes, I agree that the way it is now is a bit overkill, but it's painful to rip that out and replace it with simple string concatenation, especially at the end of {{getErrorWithDetails()}}. Is maybe {{String.format()}} the happy middle ground? In {{parseMtab()}}, this: {code} HashSet<String> validCgroups = new HashSet<>(); for (CGroupController controller : CGroupController.values()) { validCgroups.add(controller.getName()); } {code} could be this: {code} HashSet<String> validCgroups = new HashSet<>(); Collections.addAll(validCgroups, CGroupController.values()); {code} or this: {code} HashSet<String> validCgroups = new HashSet<>(Arrays.asList(CGroupController.values())); {code} In {{parseMtab()}}, this: {code} List<String> optionsList = Arrays.asList(options.split(",")); List<String> cgroupList = new LinkedList<>(); // Collect the valid subsystem names for(String cgroup: optionsList) { if (validCgroups.contains(cgroup)) { cgroupList.add(cgroup); } } {code} could be this: {code} List<String> cgroupList = Arrays.asList(options.split(",")); // Collect the valid subsystem names cgroupList.retain(validCgroups); {code} In {{mountCGroupController()}}, the message, "Trying to mount to null mount path", could be a bit more actionable. Why is the mount path null? What property should the admin change to fix it? With all the path element concatenation, have you considered using {{StringUtils.join()}} or a Guava {{Joiner}}? In {{initializeCGroupController()}}, it would also be nice to have a more actionable error message. in {{initializePreMountedCGroupController()}}, the error message doesn't tell me anything useful. Why wasn't it mounted? What should I do about it? It that a bad thing? In {{updateCGroupParam()}}, I don't see the point of the {{exceptionToThrow}}. Aren't you getting the same behavior you would if you just threw the exceptions directly (in the reverse order)? And more actionable error messages would be nice. > NM mount cpu cgroups failed on some systems > ------------------------------------------- > > Key: YARN-5301 > URL: https://issues.apache.org/jira/browse/YARN-5301 > Project: Hadoop YARN > Issue Type: Bug > Reporter: sandflee > Assignee: Miklos Szegedi > Attachments: YARN-5301.000.patch, YARN-5301.001.patch > > > on ubuntu with linux kernel 3.19, , NM start failed if enable auto mount > cgroup. try command: > ./bin/container-executor --mount-cgroups yarn-hadoop cpu=/cgroup/cpu fail > ./bin/container-executor --mount-cgroups yarn-hadoop cpu,cpuacct=/cgroup/cpu > succ -- This message was sent by Atlassian JIRA (v6.3.15#6346) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org