You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Bruce Schuchardt <bs...@pivotal.io> on 2016/12/14 16:41:39 UTC
Review Request 54752: GEODE-2213 Deadlock in GMSJoinLeaveJUnitTest
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54752/
-----------------------------------------------------------
Review request for geode, Galen O'Sullivan, Jianxia Chen, and Udo Kohlmeyer.
Bugs: GEODE-2213
https://issues.apache.org/jira/browse/GEODE-2213
Repository: geode
Description
-------
This removes the informToPendingJoinRequests invocation in recordViewRequest. I was already testing this change as I suspected that this invocation was causing new members to timeout trying to connect to the distributed system due to informToPendingJoinRequests removing join requests from the view creator's queue.
I've also made a change to DistributionConfigJUnitTest that was causing the MembershipTest suite to fail when run under IntelliJ. For some reason it does not like @Test(expected=xyz.class).
Diffs
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4ee3011b18ac309b7f3cff14f6984973b8243d86
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java 080136055feb304ebe6bfe28a0da72e67dfde21f
Diff: https://reviews.apache.org/r/54752/diff/
Testing
-------
precheckin, integration testing
Thanks,
Bruce Schuchardt
Re: Review Request 54752: GEODE-2213 Deadlock in GMSJoinLeaveJUnitTest
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54752/#review159353
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java (line 315)
<https://reviews.apache.org/r/54752/#comment230318>
This seems to work for me (expected = InternalGemFireException.class) Don't think this is a problem
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java (line 317)
<https://reviews.apache.org/r/54752/#comment230317>
I'm questioning the sanity of this test. Realistically this could be a valid address, given they have provided a fully qualified hostname, which should realistically be valid.
- Udo Kohlmeyer
On Dec. 14, 2016, 4:41 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54752/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2016, 4:41 p.m.)
>
>
> Review request for geode, Galen O'Sullivan, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Bugs: GEODE-2213
> https://issues.apache.org/jira/browse/GEODE-2213
>
>
> Repository: geode
>
>
> Description
> -------
>
> This removes the informToPendingJoinRequests invocation in recordViewRequest. I was already testing this change as I suspected that this invocation was causing new members to timeout trying to connect to the distributed system due to informToPendingJoinRequests removing join requests from the view creator's queue.
>
> I've also made a change to DistributionConfigJUnitTest that was causing the MembershipTest suite to fail when run under IntelliJ. For some reason it does not like @Test(expected=xyz.class).
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4ee3011b18ac309b7f3cff14f6984973b8243d86
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java 080136055feb304ebe6bfe28a0da72e67dfde21f
>
> Diff: https://reviews.apache.org/r/54752/diff/
>
>
> Testing
> -------
>
> precheckin, integration testing
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 54752: GEODE-2213 Deadlock in GMSJoinLeaveJUnitTest
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54752/#review159355
-----------------------------------------------------------
Ship it!
Except for the @Test exception handling, this seems to be a sane change.
- Udo Kohlmeyer
On Dec. 14, 2016, 4:41 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54752/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2016, 4:41 p.m.)
>
>
> Review request for geode, Galen O'Sullivan, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Bugs: GEODE-2213
> https://issues.apache.org/jira/browse/GEODE-2213
>
>
> Repository: geode
>
>
> Description
> -------
>
> This removes the informToPendingJoinRequests invocation in recordViewRequest. I was already testing this change as I suspected that this invocation was causing new members to timeout trying to connect to the distributed system due to informToPendingJoinRequests removing join requests from the view creator's queue.
>
> I've also made a change to DistributionConfigJUnitTest that was causing the MembershipTest suite to fail when run under IntelliJ. For some reason it does not like @Test(expected=xyz.class).
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java 4ee3011b18ac309b7f3cff14f6984973b8243d86
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java 080136055feb304ebe6bfe28a0da72e67dfde21f
>
> Diff: https://reviews.apache.org/r/54752/diff/
>
>
> Testing
> -------
>
> precheckin, integration testing
>
>
> Thanks,
>
> Bruce Schuchardt
>
>