[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:25 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse()}}, it might not make sense to send the discovery response if {{members}} is not null. It may be that we can simply return much earlier from the method if {{members}} is not null, maybe right after the call to {{readAll()}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse()}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll()}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:20 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse()}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll()}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll()}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:18 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll()}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:17 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. It may be that we can simply return much earlier from the method if {{members}} is null, maybe right after the call to {{readAll}}.
Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:12 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the discovery response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:10 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are always passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 5:10 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null. Looking at the call to {{sendDiscoveryResponse}}, I also noticed that we are passing {{false}} as the last parameter. Would it be better to pass {{is_coord}}?
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null.
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 4:00 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}, it might not make sense to send the response if {{members}} is null.
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}.
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 3:57 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
The same might also apply to the call to {{sendDiscoveryResponse}}.
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0
[JBoss JIRA] (JGRP-2288) S3_PING: Under certain conditions, subclusters fail to merge after network partition
by Nick Sawadsky (Jira)
[ https://issues.jboss.org/browse/JGRP-2288?page=com.atlassian.jira.plugin.... ]
Nick Sawadsky edited comment on JGRP-2288 at 12/13/18 3:25 PM:
---------------------------------------------------------------
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe we should check {{members == null}} earlier as well:
{code}
if(responses.isEmpty() && members == null) {
PhysicalAddress physical_addr=(PhysicalAddress)down(new Event(Event.GET_PHYSICAL_ADDRESS,local_addr));
PingData coord_data=new PingData(local_addr, true, UUID.get(local_addr), physical_addr).coord(is_coord);
write(Collections.singletonList(coord_data), cluster_name);
return;
}
{code}
was (Author: nsawadsky):
[~belaban] I've tested this out with the latest code from the 3.6 branch using S3_PING -- it's working well. Thanks very much!
I'm thinking that in the typical case, the {{writeAll()}} call will not be made, because the coordinator will find its own address in the responses: {{data == null || !data.isCoord()}} will be false. So my hope is that there shouldn't be a noticeable increase in writes to the cloud store with this change.
To ensure we don't incur extra writes to the cloud store, would it make sense to also check that {{members}} is null before doing the write? If {{members}} is non-null, then the responses will be filtered and may not include this coordinator.
{code}
if(write_data_on_find && (remove_all_files_on_view_change || remove_old_coords_on_view_change)
&& !initial_discovery && members == null && is_coord && (data == null || !data.isCoord()) && !isInfoWriterRunning()) {
// a coordinator in a separate partition may have deleted this coordinator's file
writeAll();
}
{code}
Otherwise, whenever {{TP.fetchResponsesFromDiscoveryProtocol()}} is called, we may end up writing to the cloud store.
Similarly, maybe the {{if(responses.isEmpty())}} check should be {{if ((responses.isEmpty() && members == null)}}
> S3_PING: Under certain conditions, subclusters fail to merge after network partition
> ------------------------------------------------------------------------------------
>
> Key: JGRP-2288
> URL: https://issues.jboss.org/browse/JGRP-2288
> Project: JGroups
> Issue Type: Bug
> Affects Versions: 3.6.15
> Reporter: Nick Sawadsky
> Assignee: Bela Ban
> Priority: Major
> Fix For: 4.0.17, 3.6.17
>
>
> Repro steps:
> 1. Set up a cluster of four nodes, two on one machine (Host 1) and two on another (Host 2). Let's call the nodes A, B, C, and D.
> 2. Configure all 4 nodes with S3_PING as the discovery mechanism. Set remove_all_files_on_view_change to true.
> 3. Start up nodes in the order A, B, C, D.
> 4. In the S3 bucket, there should be a single file with all four nodes listed. Node A should be flagged as the coordinator. Ensure that the UUID for node B is larger than the UUID for node C, when compared as two's complement integers. If this is not the case, shut down all nodes and restart in order. Repeat until the desired relationship is achieved. Note that with two's complement, a UUID having a first hex digit of 8 or higher is treated as negative for comparison purposes. So, for example, a UUID starting with 'a' is less than a UUID starting with 'b' which is less than a UUID starting with '1'.
> 5. On Host 1, use iptables to block all traffic going to and coming from Host 2.
> sudo iptables -A INPUT -s <Host 2 IP addr> -j DROP
> sudo iptables -A OUTPUT -d <Host 2 IP addr> -j DROP
> 6. Allow a few minutes for the nodes to detect the network partition. Eventually you should see two files in the S3 bucket.
> 7. Using Ctrl-C, stop node A.
> 8. You should soon find only a single file in the bucket, containing a single entry for node B. This is a result of the remove_all_files_on_view_change setting on S3_PING, which we set to true to avoid accumulation of old files in the bucket.
> 9. Resolve the network partition:
> sudo iptables -F OUTPUT
> sudo iptables -F INPUT
> 10. You will find that, even after many minutes, the subclusters are not merged.
> I believe the reason why the subclusters are never merged is as follows:
> - MERGE3 on nodes B, C and D uses S3_PING to find members to send INFO messages to. Each one finds only node B in the discovery file. As a result, only node B's view consistency checker has anything to work with.
> - On node B, the consistency checker can see that there are two coordinators, B and C. However, node C has a lower UUID, so node B defers to it to perform the merge. Node C never performs the merge because, as mentioned above, it is not receiving any INFO messages.
> I this this problem would affect FILE_PING as well, and other protocols derived from FILE_PING. Looking at the latest 4.x code, it appears the problem still exists there.
> I think the crux of the issue is that the coordinator on Host 2 (node C) does not re-create its discovery file after it is deleted by node B. Would it be reasonable for FILE_PING.findMembers() to create the discovery file if the node is a coordinator and the file doesn't exist?
--
This message was sent by Atlassian Jira
(v7.12.1#712002)
7 years, 7 months
- 1
- 0