Hi All,
Please take a look at this issue and my patch. Thanks!
---------- Forwarded message ----------
From: "Jason" <huzhijiang@xxxxxxxxx>
Date: Dec 10, 2014 10:23 PM
Subject: [PATCH] [TotemSRP] Ignore duplicated commit tokens in recovery mode
To: "Jason" <discuss@xxxxxxxxxxxx>
Cc: <huzhijiang@xxxxxxxxx>
In active rrp mode, commit tokens are treated as mcast data messages, thus, rrp directly delivers them to srp layer by active_mcast_recv(). This will result in duplicated commit tokens being received by srp from different heartbeat links. If node is in recovery state and has already sent out the initial orf token, those duplicated commit tokens will cause message_handler_memb_commit_token() to send initial orf token again! This is wrong because it resets the orf token content in instance->orf_token_retransmit, which breaks the token retransmission state.
Furthermore, by sending those initial orf tokens again and again, it may lead active_token_recv() to drop some subsequent orf tokens. It is OK for rrp because srp will do token retransmission, but as said above, srp retransmission state has already been broken, so finally we meet a "token lost in recovery state" condition caused by software. If token timeout value is large, then it will takes long time to create a new ring.
This can be reproduced by having two noded set to active rrp mode, with two heartbeat links. Then with one node always on, let the other one do stop/start again and again. It has a low probability to reproduce. In theory, I think, the more heartbeat links used, the more easily it can be reproduced.
This problem can be resolved by letting message_handler_memb_commit_token() to ignore duplicated commit tokens in recovery state if node (the ring representation) has already sent out the initial orf token.
Signed-off-by: Jason <huzhijiang@xxxxxxxxx>
---
exec/totemsrp.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/exec/totemsrp.c b/exec/totemsrp.c
index 95736b1..de4fce5 100644
--- a/exec/totemsrp.c
+++ b/exec/totemsrp.c
@@ -4444,6 +4444,18 @@ static int message_handler_memb_join (
return (0);
}
+static int initial_orf_token_already_sent (struct totemsrp_instance *instance)
+{
+ struct orf_token *token;
+
+ token = (struct orf_token *)(instance->orf_token_retransmit);
+ if (token->header.type == MESSAGE_TYPE_ORF_TOKEN) {
+ return 1;
+ }
+
+ return 0;
+}
+
static int message_handler_memb_commit_token (
struct totemsrp_instance *instance,
const void *msg,
@@ -4510,6 +4522,12 @@ static int message_handler_memb_commit_token (
case MEMB_STATE_RECOVERY:
if (totemip_equal (&instance->my_id.addr[0], &instance->my_ring_id.rep)) {
+
+ /* Filter out duplicated tokens */
+ if (initial_orf_token_already_sent(instance)) {
+ break;
+ }
+
log_printf (instance->totemsrp_log_level_debug,
"Sending initial ORF token");
--
1.9.4.msysgit.2
From: "Jason" <huzhijiang@xxxxxxxxx>
Date: Dec 10, 2014 10:23 PM
Subject: [PATCH] [TotemSRP] Ignore duplicated commit tokens in recovery mode
To: "Jason" <discuss@xxxxxxxxxxxx>
Cc: <huzhijiang@xxxxxxxxx>
In active rrp mode, commit tokens are treated as mcast data messages, thus, rrp directly delivers them to srp layer by active_mcast_recv(). This will result in duplicated commit tokens being received by srp from different heartbeat links. If node is in recovery state and has already sent out the initial orf token, those duplicated commit tokens will cause message_handler_memb_commit_token() to send initial orf token again! This is wrong because it resets the orf token content in instance->orf_token_retransmit, which breaks the token retransmission state.
Furthermore, by sending those initial orf tokens again and again, it may lead active_token_recv() to drop some subsequent orf tokens. It is OK for rrp because srp will do token retransmission, but as said above, srp retransmission state has already been broken, so finally we meet a "token lost in recovery state" condition caused by software. If token timeout value is large, then it will takes long time to create a new ring.
This can be reproduced by having two noded set to active rrp mode, with two heartbeat links. Then with one node always on, let the other one do stop/start again and again. It has a low probability to reproduce. In theory, I think, the more heartbeat links used, the more easily it can be reproduced.
This problem can be resolved by letting message_handler_memb_commit_token() to ignore duplicated commit tokens in recovery state if node (the ring representation) has already sent out the initial orf token.
Signed-off-by: Jason <huzhijiang@xxxxxxxxx>
---
exec/totemsrp.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/exec/totemsrp.c b/exec/totemsrp.c
index 95736b1..de4fce5 100644
--- a/exec/totemsrp.c
+++ b/exec/totemsrp.c
@@ -4444,6 +4444,18 @@ static int message_handler_memb_join (
return (0);
}
+static int initial_orf_token_already_sent (struct totemsrp_instance *instance)
+{
+ struct orf_token *token;
+
+ token = (struct orf_token *)(instance->orf_token_retransmit);
+ if (token->header.type == MESSAGE_TYPE_ORF_TOKEN) {
+ return 1;
+ }
+
+ return 0;
+}
+
static int message_handler_memb_commit_token (
struct totemsrp_instance *instance,
const void *msg,
@@ -4510,6 +4522,12 @@ static int message_handler_memb_commit_token (
case MEMB_STATE_RECOVERY:
if (totemip_equal (&instance->my_id.addr[0], &instance->my_ring_id.rep)) {
+
+ /* Filter out duplicated tokens */
+ if (initial_orf_token_already_sent(instance)) {
+ break;
+ }
+
log_printf (instance->totemsrp_log_level_debug,
"Sending initial ORF token");
--
1.9.4.msysgit.2
_______________________________________________ discuss mailing list discuss@xxxxxxxxxxxx http://lists.corosync.org/mailman/listinfo/discuss