samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jake Maes <jacob.m...@gmail.com>
Subject Re: Review Request 45190: SAMZA-910 Fix expired request test in HostAwareContainerAllocator
Date Mon, 11 Apr 2016 18:44:54 GMT


> 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?

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.


> On April 11, 2016, 6:12 p.m., Navina Ramesh wrote:
> > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java,
line 128
> > <https://reviews.apache.org/r/45190/diff/3/?file=1337905#file1337905line128>
> >
> >     "satisfied" flag here is simply indicating that we have completed running the
postConditionAssertions, right?

No, it's indicating that the condition was satisfied. That is, the expected count was observed.


As a side effect of the condition being satisfied, the assertions are run, but the 2 behaviors
should not be conflated.


- Jake


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45190/#review128184
-----------------------------------------------------------


On April 8, 2016, 11:46 p.m., Jake Maes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45190/
> -----------------------------------------------------------
> 
> (Updated April 8, 2016, 11:46 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message