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/05/10 20:52:34 UTC

Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

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

Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


Repository: geode


Description
-------

This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
  geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 

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


Testing
-------

precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.


Thanks,

Bruce Schuchardt


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Bruce Schuchardt <bs...@pivotal.io>.
It wasn't causing an issue but there's nothing that will ever clear the 
joinResponse array after it has finished joining, so if we store it in 
this method the response message would just sit taking up space forever.


Le 5/11/2016 � 10:08 AM, Hitesh Khamesra a �crit :
> This is an automatically generated e-mail. To reply, visit: 
> https://reviews.apache.org/r/47189/
>
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 
> <https://reviews.apache.org/r/47189/diff/2-3/?file=1379770#file1379770line1138> 
> (Diff revisions 2 - 3)
> protected Object sendCoordinatorFindRequest(InetSocketAddress addr, FindCoordinatorRequest request, int connectTimeout)
>
> 	
> 	1138 	
>        if  (!this.isJoined)  {
>
> Curious, What issue it was causing?
>
>
> - Hitesh Khamesra
>
>
> On May 11th, 2016, 5:05 p.m. UTC, Bruce Schuchardt wrote:
>
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo 
> Kohlmeyer.
> By Bruce Schuchardt.
>
> /Updated May 11, 2016, 5:05 p.m./
>
> *Repository: * geode
>
>
>   Description
>
> This reinstates the sending of JoinResponseMessages so that the new 
> member can get the jgroups multicast digest. The JoinResponseMessages 
> are sent after installing the new membership view, so JGroupsMessenger 
> has been changed to use MERGE_VIEW instead of SET_VIEW to install the 
> digest since it may have already received multicast messages from some 
> members.
>
>
>   Testing
>
> precheckin is underway. There is already a unit test checking that a 
> digest is added to a JoinResponseMessage so I didn't need to write one.
>
>
>   Diffs
>
>   * geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>     (88e4d496e5d89f2b84d5e755fc6471c8790ed98f)
>   * geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>     (4a54e8433ccf0bcfdacc3ccc6e790381da6e236a)
>   * geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>     (50bed13419d1ed68a405a9f5ed5d5543c3d98813)
>
> View Diff <https://reviews.apache.org/r/47189/diff/>
>


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/#review132686
-----------------------------------------------------------




geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1138)
<https://reviews.apache.org/r/47189/#comment196954>

    Curious, What issue it was causing?


- Hitesh Khamesra


On May 11, 2016, 5:05 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47189/
> -----------------------------------------------------------
> 
> (Updated May 11, 2016, 5:05 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
>   geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 
> 
> Diff: https://reviews.apache.org/r/47189/diff/
> 
> 
> Testing
> -------
> 
> precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/#review132694
-----------------------------------------------------------


Ship it!




Ship It!

- Jianxia Chen


On May 11, 2016, 5:05 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47189/
> -----------------------------------------------------------
> 
> (Updated May 11, 2016, 5:05 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
>   geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 
> 
> Diff: https://reviews.apache.org/r/47189/diff/
> 
> 
> Testing
> -------
> 
> precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/#review132729
-----------------------------------------------------------


Ship it!




Ship It!

- Udo Kohlmeyer


On May 11, 2016, 5:05 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47189/
> -----------------------------------------------------------
> 
> (Updated May 11, 2016, 5:05 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
>   geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 
> 
> Diff: https://reviews.apache.org/r/47189/diff/
> 
> 
> Testing
> -------
> 
> precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/#review132691
-----------------------------------------------------------


Ship it!




Ship It!

- Hitesh Khamesra


On May 11, 2016, 5:05 p.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47189/
> -----------------------------------------------------------
> 
> (Updated May 11, 2016, 5:05 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
>   geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 
> 
> Diff: https://reviews.apache.org/r/47189/diff/
> 
> 
> Testing
> -------
> 
> precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/
-----------------------------------------------------------

(Updated May 11, 2016, 5:05 p.m.)


Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


Repository: geode


Description
-------

This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.


Diffs (updated)
-----

  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
  geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 

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


Testing
-------

precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.


Thanks,

Bruce Schuchardt


Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47189/
-----------------------------------------------------------

(Updated May 11, 2016, 4:50 p.m.)


Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


Changes
-------

Diff updated - no need to save the JoinResponse if the new member has already finished joining the distributed system


Repository: geode


Description
-------

This reinstates the sending of JoinResponseMessages so that the new member can get the jgroups multicast digest.  The JoinResponseMessages are sent after installing the new membership view, so JGroupsMessenger has been changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it may have already received multicast messages from some members.


Diffs (updated)
-----

  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
  geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 50bed13419d1ed68a405a9f5ed5d5543c3d98813 

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


Testing
-------

precheckin is underway.  There is already a unit test checking that a digest is added to a JoinResponseMessage so I didn't need to write one.


Thanks,

Bruce Schuchardt