----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43074/#review117582 -----------------------------------------------------------
Ship it! lgtm! Please fix the nits pointed out by Jagadish. samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java (line 207) <https://reviews.apache.org/r/43074/#comment178818> Nicely refactored. Thanks! :) - Navina Ramesh On Feb. 1, 2016, 11:35 p.m., Jake Maes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43074/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2016, 11:35 p.m.) > > > Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > ------- > > Bug: > ContainerAllocator and HostAwareContainerAllocator have different > try-catch logic in their run() methods. The HostAwareContainerAllocator > try-catch is outside the while loop, which would cause it to stop allocating > containers in the event of an exception. ContainerAllocator correctly has the > try-catch inside the loop > > Fix: > Refactor the loop and try-catch to a common run() method in > AbstractContainerAllocator > Change the log type to WARN for the general exception case > Refactor some duplicate code in ContainerRequestState > > > Diffs > ----- > > > samza-yarn/src/main/java/org/apache/samza/job/yarn/AbstractContainerAllocator.java > 9ee2daccc3c44202308637207a084def81b49c09 > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerAllocator.java > 7c57a866114aabf76a36d3b7ca4c5810628e0c77 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > ab3061eae2cfc2da6681ce2034492b165d0d8b96 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/HostAwareContainerAllocator.java > ff22dbfe5537ba2a3c55c4e6063680ff4c9e55f4 > > Diff: https://reviews.apache.org/r/43074/diff/ > > > Testing > ------- > > Unit tests still pass. > > > Thanks, > > Jake Maes > >
