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 2015/10/28 23:15:50 UTC
Review Request 39738: GEODE-77: moving failure-detection port
information into the membership view
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39738/
-----------------------------------------------------------
Review request for geode and Jianxia Chen.
Repository: geode
Description
-------
We had quite a bit of convoluted code in GMSJoinLeave after implementing the tcp/ip failure detection check. This change-set moves the location of failure-detection port information out of GMSHealthMonitor and into the NetView class so that it is always carried with the view. This decouples the health monitor and join-leave and keeps the port information and member information together in a common store. This also allows us to remove the propagation of request messages from many ViewCreator methods and remove the port-lookup code from join-leave.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java cb61a1ba6e90939f026ddffc1983d274b50e3e15
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java d89ba39265478583a407f4803a40d08302c55af3
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 628e416a3718bb02b9657d716017e7b0e8658368
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 5633424b69bb5b35242015da1d557af358b2f13d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/InstallViewMessage.java 1afdf401ce93c03af1da929f0cd6712c152d0211
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java 952b20e3fda830f86729d60c7f20513d69dae7eb
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java 77b72c348be93c1cda623694b3336725748ceefe
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ae7ee16b818b09a7315210e5c427486ce841ab60
gemfire-core/src/main/java/com/gemstone/gemfire/internal/util/PluckStacks.java 15a64d7441c09e178eaef40598f32226a69c8d00
gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java 26e85869f920fdc6044c059709012e3a9056dfab
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 8779c6d7fe66ef55cf293c19f5f66b7a3ed20823
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 4e3293221e59d45cc8b2e9486b9e008b34225231
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java 2b9d451a5c24fbff740c97658e1e58eef34f5dc8
Diff: https://reviews.apache.org/r/39738/diff/
Testing
-------
unit testing (underway)
Thanks,
Bruce Schuchardt
Re: Review Request 39738: GEODE-77: moving failure-detection port
information into the membership view
Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39738/#review104392
-----------------------------------------------------------
I am not sure whether changing NetView has any impact on backward compatibility.
sanctionedSerializables.txt and sanctionedDataSerializables.txt should be changed as well. Since NetView, JoinRequestMessage, JoinResponseMessage and InstallViewMessage are all changed.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
<https://reviews.apache.org/r/39738/#comment162662>
Why this is removed?
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java (line 98)
<https://reviews.apache.org/r/39738/#comment162663>
Shall we do the same for other non-copy constructors? So that failureDetectionPorts are initialized properly.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java (line 534)
<https://reviews.apache.org/r/39738/#comment162666>
failure detection server thread
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 659)
<https://reviews.apache.org/r/39738/#comment162667>
Remove this line.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 660)
<https://reviews.apache.org/r/39738/#comment162668>
Remove this line as well.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1898)
<https://reviews.apache.org/r/39738/#comment162669>
For every iteration, it will use the same port for different mbr. Is this expected?
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java (line 51)
<https://reviews.apache.org/r/39738/#comment162657>
" socketPort:" should be " failureDetectionPort:"
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java (line 81)
<https://reviews.apache.org/r/39738/#comment162670>
Should be named setFailureDetectionPort
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java (line 95)
<https://reviews.apache.org/r/39738/#comment162658>
This function should be removed.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java (line 110)
<https://reviews.apache.org/r/39738/#comment162659>
This should be removed as well.
gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java (line 329)
<https://reviews.apache.org/r/39738/#comment162660>
What is this code for?
- Jianxia Chen
On Oct. 28, 2015, 10:20 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39738/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 10:20 p.m.)
>
>
> Review request for geode and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> We had quite a bit of convoluted code in GMSJoinLeave after implementing the tcp/ip failure detection check. This change-set moves the location of failure-detection port information out of GMSHealthMonitor and into the NetView class so that it is always carried with the view. This decouples the health monitor and join-leave and keeps the port information and member information together in a common store. This also allows us to remove the propagation of request messages from many ViewCreator methods and remove the port-lookup code from join-leave.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java cb61a1ba6e90939f026ddffc1983d274b50e3e15
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java d89ba39265478583a407f4803a40d08302c55af3
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 628e416a3718bb02b9657d716017e7b0e8658368
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 5633424b69bb5b35242015da1d557af358b2f13d
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/InstallViewMessage.java 1afdf401ce93c03af1da929f0cd6712c152d0211
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java 952b20e3fda830f86729d60c7f20513d69dae7eb
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java 77b72c348be93c1cda623694b3336725748ceefe
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ae7ee16b818b09a7315210e5c427486ce841ab60
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/util/PluckStacks.java 15a64d7441c09e178eaef40598f32226a69c8d00
> gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java 26e85869f920fdc6044c059709012e3a9056dfab
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 8779c6d7fe66ef55cf293c19f5f66b7a3ed20823
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 4e3293221e59d45cc8b2e9486b9e008b34225231
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java 2b9d451a5c24fbff740c97658e1e58eef34f5dc8
>
> Diff: https://reviews.apache.org/r/39738/diff/
>
>
> Testing
> -------
>
> unit testing (underway)
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 39738: GEODE-77: moving failure-detection port
information into the membership view
Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39738/#review104452
-----------------------------------------------------------
Ship it!
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java (line 42)
<https://reviews.apache.org/r/39738/#comment162717>
private
- Jianxia Chen
On Oct. 29, 2015, 4:34 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39738/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 4:34 p.m.)
>
>
> Review request for geode and Jianxia Chen.
>
>
> Repository: geode
>
>
> Description
> -------
>
> We had quite a bit of convoluted code in GMSJoinLeave after implementing the tcp/ip failure detection check. This change-set moves the location of failure-detection port information out of GMSHealthMonitor and into the NetView class so that it is always carried with the view. This decouples the health monitor and join-leave and keeps the port information and member information together in a common store. This also allows us to remove the propagation of request messages from many ViewCreator methods and remove the port-lookup code from join-leave.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java cb61a1ba6e90939f026ddffc1983d274b50e3e15
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java d89ba39265478583a407f4803a40d08302c55af3
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 628e416a3718bb02b9657d716017e7b0e8658368
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 5633424b69bb5b35242015da1d557af358b2f13d
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/InstallViewMessage.java 1afdf401ce93c03af1da929f0cd6712c152d0211
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java 952b20e3fda830f86729d60c7f20513d69dae7eb
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java 77b72c348be93c1cda623694b3336725748ceefe
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ae7ee16b818b09a7315210e5c427486ce841ab60
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/InternalDataSerializer.java 8e864c9dfca76c22a5c17c19d2cd5415e99b8006
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/util/PluckStacks.java 15a64d7441c09e178eaef40598f32226a69c8d00
> gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java 26e85869f920fdc6044c059709012e3a9056dfab
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 8779c6d7fe66ef55cf293c19f5f66b7a3ed20823
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 4e3293221e59d45cc8b2e9486b9e008b34225231
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java 2b9d451a5c24fbff740c97658e1e58eef34f5dc8
>
> Diff: https://reviews.apache.org/r/39738/diff/
>
>
> Testing
> -------
>
> unit testing (underway)
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 39738: GEODE-77: moving failure-detection port
information into the membership view
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39738/
-----------------------------------------------------------
(Updated Oct. 29, 2015, 4:34 p.m.)
Review request for geode and Jianxia Chen.
Changes
-------
diff updated to address comments
Repository: geode
Description
-------
We had quite a bit of convoluted code in GMSJoinLeave after implementing the tcp/ip failure detection check. This change-set moves the location of failure-detection port information out of GMSHealthMonitor and into the NetView class so that it is always carried with the view. This decouples the health monitor and join-leave and keeps the port information and member information together in a common store. This also allows us to remove the propagation of request messages from many ViewCreator methods and remove the port-lookup code from join-leave.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java cb61a1ba6e90939f026ddffc1983d274b50e3e15
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java d89ba39265478583a407f4803a40d08302c55af3
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 628e416a3718bb02b9657d716017e7b0e8658368
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 5633424b69bb5b35242015da1d557af358b2f13d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/InstallViewMessage.java 1afdf401ce93c03af1da929f0cd6712c152d0211
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java 952b20e3fda830f86729d60c7f20513d69dae7eb
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java 77b72c348be93c1cda623694b3336725748ceefe
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ae7ee16b818b09a7315210e5c427486ce841ab60
gemfire-core/src/main/java/com/gemstone/gemfire/internal/InternalDataSerializer.java 8e864c9dfca76c22a5c17c19d2cd5415e99b8006
gemfire-core/src/main/java/com/gemstone/gemfire/internal/util/PluckStacks.java 15a64d7441c09e178eaef40598f32226a69c8d00
gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java 26e85869f920fdc6044c059709012e3a9056dfab
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 8779c6d7fe66ef55cf293c19f5f66b7a3ed20823
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 4e3293221e59d45cc8b2e9486b9e008b34225231
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java 2b9d451a5c24fbff740c97658e1e58eef34f5dc8
Diff: https://reviews.apache.org/r/39738/diff/
Testing
-------
unit testing (underway)
Thanks,
Bruce Schuchardt
Re: Review Request 39738: GEODE-77: moving failure-detection port
information into the membership view
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39738/
-----------------------------------------------------------
(Updated Oct. 28, 2015, 10:20 p.m.)
Review request for geode and Jianxia Chen.
Repository: geode
Description
-------
We had quite a bit of convoluted code in GMSJoinLeave after implementing the tcp/ip failure detection check. This change-set moves the location of failure-detection port information out of GMSHealthMonitor and into the NetView class so that it is always carried with the view. This decouples the health monitor and join-leave and keeps the port information and member information together in a common store. This also allows us to remove the propagation of request messages from many ViewCreator methods and remove the port-lookup code from join-leave.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java cb61a1ba6e90939f026ddffc1983d274b50e3e15
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java d89ba39265478583a407f4803a40d08302c55af3
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 628e416a3718bb02b9657d716017e7b0e8658368
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java 5633424b69bb5b35242015da1d557af358b2f13d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/InstallViewMessage.java 1afdf401ce93c03af1da929f0cd6712c152d0211
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinRequestMessage.java 952b20e3fda830f86729d60c7f20513d69dae7eb
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/JoinResponseMessage.java 77b72c348be93c1cda623694b3336725748ceefe
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ae7ee16b818b09a7315210e5c427486ce841ab60
gemfire-core/src/main/java/com/gemstone/gemfire/internal/util/PluckStacks.java 15a64d7441c09e178eaef40598f32226a69c8d00
gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java 26e85869f920fdc6044c059709012e3a9056dfab
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 8779c6d7fe66ef55cf293c19f5f66b7a3ed20823
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 4e3293221e59d45cc8b2e9486b9e008b34225231
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java 2b9d451a5c24fbff740c97658e1e58eef34f5dc8
Diff: https://reviews.apache.org/r/39738/diff/
Testing
-------
unit testing (underway)
Thanks,
Bruce Schuchardt