You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Hitesh Khamesra <hk...@pivotal.io> on 2016/05/02 18:36:59 UTC
Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/
-----------------------------------------------------------
Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
Repository: geode
Description
-------
added synchronization with copyOnread
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
Diff: https://reviews.apache.org/r/46896/diff/
Testing
-------
Thanks,
Hitesh Khamesra
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131367
-----------------------------------------------------------
Ship it!
Ship It!
- Udo Kohlmeyer
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131528
-----------------------------------------------------------
I like this a lot better
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
<https://reviews.apache.org/r/46896/#comment195541>
I think an interrupt should terminate the wait. How about changing the method to declare "throws InterruptedException" and handle it in the method that invokes this one?
- Bruce Schuchardt
On May 3, 2016, 4:46 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 3, 2016, 4:46 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131714
-----------------------------------------------------------
Ship it!
- Bruce Schuchardt
On May 4, 2016, 5:12 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 4, 2016, 5:12 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/
-----------------------------------------------------------
(Updated May 4, 2016, 5:12 p.m.)
Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
Changes
-------
Now WaitForResponses method throws Interrup Exception which is handled by caller and it terminates ViewCreator thread.
Repository: geode
Description
-------
added synchronization with copyOnread
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
Diff: https://reviews.apache.org/r/46896/diff/
Testing
-------
Thanks,
Hitesh Khamesra
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/
-----------------------------------------------------------
(Updated May 3, 2016, 4:46 p.m.)
Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
Changes
-------
Updated patch with copyOnRead.
1. synchronized "getUnresponsiveMembers" method and now this returns copy of set.
2. Simplified "waitForResponses" method and this also returns copy of set.
Repository: geode
Description
-------
added synchronization with copyOnread
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
Diff: https://reviews.apache.org/r/46896/diff/
Testing
-------
Thanks,
Hitesh Khamesra
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
> On May 2, 2016, 11:06 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1595
> > <https://reviews.apache.org/r/46896/diff/2/?file=1369119#file1369119line1595>
> >
> > I still don't like this. getUnresponsiveMembers() is used by the view creator to get the collection and then it proceeds to modify it multiple times. I think that internally the reply processor should continue to use the collection directly and that getUnresponsiveMembers() should synchronize and return a copy of the collection. That will result in a lot fewer objects being created and will keep the view-creator thread from modifying the collection that is internal to the reply processor.
>>That will result in a lot fewer objects being created and will keep the view-creator thread from modifying the collection that is internal to the reply processor.
So we think this is another issue, where viewCreator is modifying the "Set" of replyProcessor.
That means orignal fix is right way to go. "waitForResponses" function also returns same "set" that's why I made "copyOnRead" there as well.
- Hitesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131408
-----------------------------------------------------------
On May 2, 2016, 10:34 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 10:34 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131408
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1595)
<https://reviews.apache.org/r/46896/#comment195367>
I still don't like this. getUnresponsiveMembers() is used by the view creator to get the collection and then it proceeds to modify it multiple times. I think that internally the reply processor should continue to use the collection directly and that getUnresponsiveMembers() should synchronize and return a copy of the collection. That will result in a lot fewer objects being created and will keep the view-creator thread from modifying the collection that is internal to the reply processor.
- Bruce Schuchardt
On May 2, 2016, 10:34 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 10:34 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/
-----------------------------------------------------------
(Updated May 2, 2016, 10:34 p.m.)
Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
Changes
-------
Darrel Suggested that update patch with CopyOnWrite and that will be one liner change.
Repository: geode
Description
-------
added synchronization with copyOnread
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
Diff: https://reviews.apache.org/r/46896/diff/
Testing
-------
Thanks,
Hitesh Khamesra
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1699
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1699>
> >
> > I don't understand why this needs to be called here as well as in the while block. You could initialize it here,but I don't think we need the extra (new HashSet()) call here as well. This call is not required.
It may not go into while loop. Thus needs to initialize it
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1703
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1703>
> >
> > This can potentially cause a huge amount of unnecessary garbage. Each time this method is called we create a new HashSet.
i don't think so, it will called after 1 second period or when new member will be added
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1741
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1741>
> >
> > I don't think we need synchronized here. There is no chance for any concurrent modification here. Even with mutliple threads this is still just a getter invocation, there is no danger to concurrently modify anything.
I think this is the current issue where one thread is itersating over collection and other is adding.
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1742
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1742>
> >
> > I don't think we need to explicitly create a new HashSet for a getter. If this set needs to be "cloned" then make it the responsibility of the method invoking this function to do so.
We can do that way as well. But then every accesor has to do.
- Hitesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131357
-----------------------------------------------------------
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Darrel Schneider <ds...@pivotal.io>.
> On May 2, 2016, 12:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1742
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1742>
> >
> > I don't think we need to explicitly create a new HashSet for a getter. If this set needs to be "cloned" then make it the responsibility of the method invoking this function to do so.
>
> Hitesh Khamesra wrote:
> We can do that way as well. But then every accesor has to do.
>
> Udo Kohlmeyer wrote:
> I'm torn on this one... I don't see a need to force a new collection everytime, but on the other hand.. being safe hasn't killed anyone.
Have you considered using com.gemstone.gemfire.internal.CopyOnWriteHashSet instead of HashSet?
- Darrel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131357
-----------------------------------------------------------
On May 2, 2016, 9:36 a.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 9:36 a.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Udo Kohlmeyer <uk...@gmail.com>.
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1703
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1703>
> >
> > This can potentially cause a huge amount of unnecessary garbage. Each time this method is called we create a new HashSet.
>
> Hitesh Khamesra wrote:
> i don't think so, it will called after 1 second period or when new member will be added
We don't know how long something might run. Which means, it might run a few times. I'm just conscious that it might run multiple times, and creating new HashSets each time just creates more garbage.
But happy for you to leave it if you think it will not cause issues.
> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, line 1742
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1742>
> >
> > I don't think we need to explicitly create a new HashSet for a getter. If this set needs to be "cloned" then make it the responsibility of the method invoking this function to do so.
>
> Hitesh Khamesra wrote:
> We can do that way as well. But then every accesor has to do.
I'm torn on this one... I don't see a need to force a new collection everytime, but on the other hand.. being safe hasn't killed anyone.
- Udo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131357
-----------------------------------------------------------
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131357
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1699)
<https://reviews.apache.org/r/46896/#comment195284>
I don't understand why this needs to be called here as well as in the while block. You could initialize it here,but I don't think we need the extra (new HashSet()) call here as well. This call is not required.
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1703)
<https://reviews.apache.org/r/46896/#comment195283>
This can potentially cause a huge amount of unnecessary garbage. Each time this method is called we create a new HashSet.
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1741)
<https://reviews.apache.org/r/46896/#comment195281>
I don't think we need synchronized here. There is no chance for any concurrent modification here. Even with mutliple threads this is still just a getter invocation, there is no danger to concurrently modify anything.
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1742)
<https://reviews.apache.org/r/46896/#comment195285>
I don't think we need to explicitly create a new HashSet for a getter. If this set needs to be "cloned" then make it the responsibility of the method invoking this function to do so.
- Udo Kohlmeyer
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Hitesh Khamesra <hk...@pivotal.io>.
> On May 2, 2016, 5:01 p.m., Bruce Schuchardt wrote:
> > I can only see one place where this collection is being accessed outside of synchronization: waitForResults() returns it instead of a copy of the collection.
> >
> > Can't we just make a copy of the collection under synchronization in that method instead of changing checkIfDone()?
"getUnresponsiveMembers()" method also need to consider as it returns "notRepliedYet" set, which is called from "prepareAndSendView" method.
- Hitesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131330
-----------------------------------------------------------
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131330
-----------------------------------------------------------
I can only see one place where this collection is being accessed outside of synchronization: waitForResults() returns it instead of a copy of the collection.
Can't we just make a copy of the collection under synchronization in that method instead of changing checkIfDone()?
- Bruce Schuchardt
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got
java.util.ConcurrentModificationException while updating log message
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46896/#review131374
-----------------------------------------------------------
I think you should remove the hashset creation and just fix the waitForResponses problem that's allowing the collection to escape from the reply processor
- Bruce Schuchardt
On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 4:36 p.m.)
>
>
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> added synchronization with copyOnread
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 9f5648b
>
> Diff: https://reviews.apache.org/r/46896/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>