You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2016/06/10 00:03:43 UTC

Review Request 48525: GEODE-1422: Removing check to skip enqueuing temp events

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

Review request for geode, anilkumar gingade, Jason Huynh, and nabarun nag.


Repository: geode


Description
-------

This check was performed outside of synchronizing on the lock, which
means that some events could be added to the temp events after the
check. This caused the test to fail due to temp events being left in the
map.

Also, fixing two race conditions in testParallelPropagationSenderStartAfterStop_Scenario2

1) It did some puts and stopped the sender without waiting for the
puts to complete. So they may not be available on the remote side

2) It started some puts asynchronously, and then started the sender
later. The puts performed before the sender started may not be present
on the remote side.


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/ParallelAsyncEventQueueImpl.java aa3e71cb222a610f7caf297e8c1990778da40c94 
  geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderImpl.java fa21b18e8de2a500130a4cb7f179b82b7da681e0 
  geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderOperationsDUnitTest.java 3f0329a21a92ca7902a3fdaaf6ad0e0b4ef59224 

Diff: https://reviews.apache.org/r/48525/diff/


Testing
-------


Thanks,

Dan Smith


Re: Review Request 48525: GEODE-1422: Removing check to skip enqueuing temp events

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48525/#review137055
-----------------------------------------------------------


Fix it, then Ship it!




Ship It!


geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/ParallelAsyncEventQueueImpl.java (line 112)
<https://reviews.apache.org/r/48525/#comment202191>

    Something may be wrong with our IDE formatters, they are adding spaces with new line...I did see this with my eclipse IDE...


- anilkumar gingade


On June 10, 2016, 12:03 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48525/
> -----------------------------------------------------------
> 
> (Updated June 10, 2016, 12:03 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This check was performed outside of synchronizing on the lock, which
> means that some events could be added to the temp events after the
> check. This caused the test to fail due to temp events being left in the
> map.
> 
> Also, fixing two race conditions in testParallelPropagationSenderStartAfterStop_Scenario2
> 
> 1) It did some puts and stopped the sender without waiting for the
> puts to complete. So they may not be available on the remote side
> 
> 2) It started some puts asynchronously, and then started the sender
> later. The puts performed before the sender started may not be present
> on the remote side.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/ParallelAsyncEventQueueImpl.java aa3e71cb222a610f7caf297e8c1990778da40c94 
>   geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderImpl.java fa21b18e8de2a500130a4cb7f179b82b7da681e0 
>   geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderOperationsDUnitTest.java 3f0329a21a92ca7902a3fdaaf6ad0e0b4ef59224 
> 
> Diff: https://reviews.apache.org/r/48525/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>