> On April 11, 2016, 6:12 p.m., Navina Ramesh wrote: > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocator.java, > > line 69 > > <https://reviews.apache.org/r/45190/diff/3/?file=1337902#file1337902line69> > > > > Why should config be static? > > Jake Maes wrote: > To ensure that it gets intialized before getConfig() is called. But it > looks like getConfig() was unnecessarily complicated, so I just fixed that > and made config final. No more race condition. It could still be static since > it's essentially a constant, but that's splitting hairs.
Ok. - Navina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45190/#review128184 ----------------------------------------------------------- On April 11, 2016, 6:45 p.m., Jake Maes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45190/ > ----------------------------------------------------------- > > (Updated April 11, 2016, 6:45 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > SAMZA-910 Fix expired request test in HostAwareContainerAllocator > > Summary of changes: > 1. Remove the last sleep() from HostAwareContainerAllocator > 2. Fix a silent failure in testRerequestOnAnyHostIfContainerStartFails by > setting the neededContainers to 1 before running the test. > 3. Update MockContainerListener so assertion failures in other threads are > thrown in the main thread to fail the test. (no silent failures) This should > help troubleshoot the tests if there are any remaining issues. > 4. Rename obscure hostnames to make it easier to reason about the tests. > > > Diffs > ----- > > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocator.java > b253f98f7258bb611e1ad6672f74b07ab7e20b70 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocatorCommon.java > PRE-CREATION > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestHostAwareContainerAllocator.java > 93e430b6ee986b06ecdac4979552d774724a1fbd > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java > cb82cccf75b54cfbefd586700e8283cb41173833 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerRequestState.java > 879a7d0d06b087cfe0417f3fa5801b43ac7fc458 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerUtil.java > 2f9669f8b7e77abb65b244ccd067ae7ab1f245c3 > > Diff: https://reviews.apache.org/r/45190/diff/ > > > Testing > ------- > > Ran build and check-all on both of my machines twice. I don't see any > sporadic failures anymore. > > > Thanks, > > Jake Maes > >
