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/10/12 18:12:15 UTC

Review Request 52796: GEODE-1981: Synchronizing the ResultCollector when adding results

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

Review request for geode, Barry Oglesby and nabarun nag.


Repository: geode


Description
-------

When executing a function from a client, we can be adding results to the
result collector from multiple threads. One code path was already
synchronizing on the collector when adding results. However, if the
function returned an exception we were not synchronizing.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java 6597b680227fb47492dc973baecffd504d6cdf68 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java ae08ac0affdef7f2270f68da5622194be8fd009f 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionSingleHopOp.java 51ea8e4fbc64acbbd1165856a2dd09704d63e857 

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


Testing
-------

Run the test in this bug 500 times with and without this fix. Without the fix it failed 11 times, which the fix it passed every time. Running precheckin on these changes.


Thanks,

Dan Smith


Re: Review Request 52796: GEODE-1981: Synchronizing the ResultCollector when adding results

Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52796/#review152387
-----------------------------------------------------------



Based on feedback from Naba, I'll rework this to just wrap the result collector in a synchronized result collector, to avoid places where we might be accessing this thing without synchronization.

- Dan Smith


On Oct. 12, 2016, 6:12 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52796/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2016, 6:12 p.m.)
> 
> 
> Review request for geode, Barry Oglesby and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> When executing a function from a client, we can be adding results to the
> result collector from multiple threads. One code path was already
> synchronizing on the collector when adding results. However, if the
> function returned an exception we were not synchronizing.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java 6597b680227fb47492dc973baecffd504d6cdf68 
>   geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java ae08ac0affdef7f2270f68da5622194be8fd009f 
>   geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionSingleHopOp.java 51ea8e4fbc64acbbd1165856a2dd09704d63e857 
> 
> Diff: https://reviews.apache.org/r/52796/diff/
> 
> 
> Testing
> -------
> 
> Run the test in this bug 500 times with and without this fix. Without the fix it failed 11 times, which the fix it passed every time. Running precheckin on these changes.
> 
> 
> Thanks,
> 
> Dan Smith
> 
>