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/08/05 16:52:32 UTC
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/
-----------------------------------------------------------
(Updated Aug. 5, 2016, 4:52 p.m.)
Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
Changes
-------
Updated patch as Bruce suggested
Repository: geode
Description
-------
Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/InternalDistributedMember.java 067b71b
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
Diff: https://reviews.apache.org/r/49962/diff/
Testing
-------
Thanks,
Hitesh Khamesra
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/#review145138
-----------------------------------------------------------
Ship it!
Ship It!
- Bruce Schuchardt
On ao�t 5, 2016, 4:52 apr�s-midi, Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> -----------------------------------------------------------
>
> (Updated ao�t 5, 2016, 4:52 apr�s-midi)
>
>
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/InternalDistributedMember.java 067b71b
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
>
> Diff: https://reviews.apache.org/r/49962/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Hitesh Khamesra <hk...@pivotal.io>.
> On Aug. 22, 2016, 3:29 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java, line 895
> > <https://reviews.apache.org/r/49962/diff/3/?file=1470397#file1470397line895>
> >
> > I don't think this can be transparent, but at the same time do we really need static methods on GMSMember for this when there are already instance methods? I also prefer the method names in InternalDistributedMember (write/readEssentialData) because "shallow" to me means not recursing into contained objects, such as "shallow copy".
agree..
- Hitesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/#review146361
-----------------------------------------------------------
On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2016, 9:25 p.m.)
>
>
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java 87409c5
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 080bdb3
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
>
> Diff: https://reviews.apache.org/r/49962/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/#review146361
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java (line 895)
<https://reviews.apache.org/r/49962/#comment212757>
I don't think this can be transparent, but at the same time do we really need static methods on GMSMember for this when there are already instance methods? I also prefer the method names in InternalDistributedMember (write/readEssentialData) because "shallow" to me means not recursing into contained objects, such as "shallow copy".
- Bruce Schuchardt
On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2016, 9:25 p.m.)
>
>
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java 87409c5
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 080bdb3
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
>
> Diff: https://reviews.apache.org/r/49962/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Udo Kohlmeyer <uk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/#review146276
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java (line 895)
<https://reviews.apache.org/r/49962/#comment212651>
I believe that this static method invocation should not happen here. I think that the serialization should be transparent. If we start special casing it this, it can quickly become unmanageable.
- Udo Kohlmeyer
On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2016, 9:25 p.m.)
>
>
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java 87409c5
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 080bdb3
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
>
> Diff: https://reviews.apache.org/r/49962/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/#review145409
-----------------------------------------------------------
It looks good but you need to change the GMSMember reference in JoinLeave to NetMember so that we don't tie this interface to the implementation classes.
Fix it, then ship it!
- Bruce Schuchardt
On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2016, 9:25 p.m.)
>
>
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java 87409c5
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 080bdb3
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
>
> Diff: https://reviews.apache.org/r/49962/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hitesh Khamesra
>
>
Re: Review Request 49962: Removed extra fields from distributedmember
while serialization/de
Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49962/
-----------------------------------------------------------
(Updated Aug. 10, 2016, 9:25 p.m.)
Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
Changes
-------
Updated patch. Now we look preparedview as well to find the InternalDistributedMember from view. Some other calenup.
Repository: geode
Description
-------
Removed extra fields from distributedmember while serialization/de. Planning to do this in udp-security branch.
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java d5d0b8e
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java 87409c5
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 080bdb3
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java 5c0a327
geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java 7eef594
Diff: https://reviews.apache.org/r/49962/diff/
Testing
-------
Thanks,
Hitesh Khamesra