> On March 14, 2016, 6:51 a.m., Navina Ramesh wrote: > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java, > > line 149 > > <https://reviews.apache.org/r/44775/diff/1/?file=1297033#file1297033line149> > > > > I remember specifically not simplifying the conditions so that we can > > enable debug logging to troubleshoot any issues with the allocation logic. > > I do agree there is some amount of readability issue here. But I feel that > > the log lines are much useful until we stabilize the allocator.
Having said that, I would like to hear the comments from others. I also think we need to add unit tests specifically for the conditions that we simplify / optimize, because our unit test coverage is pretty low at this point :( - Navina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44775/#review123359 ----------------------------------------------------------- On March 14, 2016, 6:02 a.m., Jagadish Venkatraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44775/ > ----------------------------------------------------------- > > (Updated March 14, 2016, 6:02 a.m.) > > > Review request for samza, Boris Shkolnik, Jake Maes, Yi Pan (Data > Infrastructure), Navina Ramesh, and Xinyu Liu. > > > Repository: samza > > > Description > ------- > > The current container allocator has some repetitive checks - when it decides > to enqueue a container to ANY_HOST. I believe it is a good idea to eliminate > those repetitive checks and refactor the allocation logic. It will help in > long-term maintainability. > > > Diffs > ----- > > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > 3e3f48ce2b5c0802e8c7c4f09b632df2d2265c12 > > Diff: https://reviews.apache.org/r/44775/diff/ > > > Testing > ------- > > All unit tests still pass. > > > Thanks, > > Jagadish Venkatraman > >
