> On Dec. 11, 2013, 3:33 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala, > > line 32 > > <https://reviews.apache.org/r/16194/diff/1/?file=396745#file396745line32> > > > > There might be some docs that mention this environment variable. > > Probably worth a grep to replace with the new name.
Did. Didn't see any. > On Dec. 11, 2013, 3:33 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, > > line 339 > > <https://reviews.apache.org/r/16194/diff/1/?file=396747#file396747line339> > > > > Stylistic: strip extra white space line. done > On Dec. 11, 2013, 3:33 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 134 > > <https://reviews.apache.org/r/16194/diff/1/?file=396750#file396750line134> > > > > I think this might be a misnomer that is not your fault, but that you > > inherited from my poor naming conventions in the Samza YARN code. > > > > What this method is actually used for, as far as I can tell, is to > > figure out which input SSPs a given samza CONTAINER is responsible for > > processing (not a task). It's unfortunate, but Samza's YARN code still > > refers to a container as a task. Therefore, taskId is an id for a YARN > > container, and taskCount is the total number of containers > > (yarn.container.count). > > > > Your call whether you want to stick with existing convention, update > > all mentions in the YARN AM to refer to container instead of task, or just > > update this chunk of code to do the right thing. Changed it on this method (and method params). We can update the rest in another JIRA. > On Dec. 11, 2013, 3:33 p.m., Chris Riccomini wrote: > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala, > > line 130 > > <https://reviews.apache.org/r/16194/diff/1/?file=396753#file396753line130> > > > > This is all using the wrong terminology. Should be "container" not > > "task". See comment above. Up to you if you want to fix, or open a new > > JIRA, or what. done. > On Dec. 11, 2013, 3:33 p.m., Chris Riccomini wrote: > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala, > > line 136 > > <https://reviews.apache.org/r/16194/diff/1/?file=396753#file396753line136> > > > > Is this actually used anywhere? The semantics have changed. Before, > > partitions.size would give you the count of all unique partitions (e.g. two > > input streams with 4 partitions each = 4 partitions). > > > > With your change, the total partitions now means the total number of > > SSPs that the container is processing (e.g. two input streams with 4 > > partitions each = 8 partitions). No. Had meant to remove it. Done now. - Jakob ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16194/#review30233 ----------------------------------------------------------- On Dec. 11, 2013, 1 p.m., Jakob Homan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16194/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2013, 1 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-82 > https://issues.apache.org/jira/browse/SAMZA-82 > > > Repository: samza > > > Description > ------- > > Fix Samza-82 > > > Diffs > ----- > > samza-api/src/main/java/org/apache/samza/job/CommandBuilder.java 934423b > samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala > b6efe46 > samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala 9c4370f > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > aefec27 > samza-core/src/main/scala/org/apache/samza/job/ShellCommandBuilder.scala > c67e46d > samza-core/src/main/scala/org/apache/samza/job/local/LocalJobFactory.scala > ddb119b > samza-core/src/main/scala/org/apache/samza/util/Util.scala 6b2ec49 > samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala PRE-CREATION > > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala > bc94f6a > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala > 9f4db17 > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterTaskManager.scala > ee3ffef > > Diff: https://reviews.apache.org/r/16194/diff/ > > > Testing > ------- > > unit and manual > > > Thanks, > > Jakob Homan > >
