----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51633/#review151519 -----------------------------------------------------------
Overall, the patch looks great! This is exciting given that Samza can support scheduling based on tags. For example, jobs with rocksdb can be assigned to nodes with SSDs. Can you please add some detail on testing this feature? What was the label setup of the cluster? (for example: Did we use an exclusive node label?), How many node labels? How many containers were requested for the job? samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java (line 187) <https://reviews.apache.org/r/51633/#comment219938> nit: containerLabel so that we distinguish it from the AM label. samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java (line 207) <https://reviews.apache.org/r/51633/#comment219940> Question: How does the node-label feature inter-play with the preferred host feature in Yarn? (Samza leverages this for host-affinity) Does the label take precedence over the host? (or vice-versa). samza-yarn/src/main/scala/org/apache/samza/job/yarn/ClientHelper.scala (line 97) <https://reviews.apache.org/r/51633/#comment219937> nit: rename this to appMasterLabel (to avoid a conflict with containerLabel). - Jagadish Venkatraman On Sept. 5, 2016, 5:16 p.m., Maxim Logvinenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51633/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2016, 5:16 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-1013 > https://issues.apache.org/jira/browse/SAMZA-1013 > > > Repository: samza > > > Description > ------- > > YARN Node labels were introduced in Hadoop version 2.6, which allows to group > nodes with similar characteristics and allows applications to specify where > to run. This patch adds support for YARN node labels in Samza. > > In this implementation, node labels are defined directly in yarnConfig in > YarnClusterResourceManager. It might be better to have node labels as a part > of SamzaResourceRequest and SamzaResource classes, but > org.apache.hadoop.yarn.api.records.Container class doesn't contain node label > and hence we have nothing to pass to the SamzaResource constructor in > onContainersAllocated method of YarnClusterResourceManager class. > > > Diffs > ----- > > samza-yarn/src/main/java/org/apache/samza/config/YarnConfig.java 8f2dc48 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java > 96d3d7c > samza-yarn/src/main/scala/org/apache/samza/job/yarn/ClientHelper.scala > 0998c43 > > Diff: https://reviews.apache.org/r/51633/diff/ > > > Testing > ------- > > > Thanks, > > Maxim Logvinenko > >
