govind-menon commented on a change in pull request #3237: STORM-3259: Adds NUMA
awareness to enable worker pinning
URL: https://github.com/apache/storm/pull/3237#discussion_r403085070
##########
File path:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/BasicContainer.java
##########
@@ -665,8 +666,10 @@ protected String javaCmd(String cmd) {
commandList.addAll(classPathParams);
commandList.add(getWorkerMain(topoVersion));
commandList.add(topologyId);
+ if (numaId != null) {
+ supervisorId += Constants.NUMA_ID_SEPARATOR + numaId;
Review comment:
I'm putting this back here - there could be/are multiple implementations for
how a ResourceIsolation manager could implement NUMA pinning but the supervisor
ID MUST contain the numa id in the current implementation. But this supervisor
id is effectively only used when the worker is launched. I feel it is
appropriate to keep it here so that it's explicitly used as opposed to a super
constructor where it can be missed when future changes are maded
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
