dlm_controld: handle the case of network transient disconnection

DLM would be stuck in "need fencing" state, although cluster can regain quorum very quickly after a network transient disconnection.

It's possible that this process happens within one monoclock. It means "cluster_quorate_monotime" can eqaul "node->daemon_rem_time". We now skip this chance of telling corosync to kill cluster for stateful merge. As a result, any fencing cannot proceed further.

---
 dlm_controld/daemon_cpg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 356e80d..cd8a4e2 100644
@@ -1695,7 +1695,7 @@ static void receive_protocol(struct dlm_header *hd, int len)
 		node->stateful_merge = 1;
 
 		if (cluster_quorate && node->daemon_rem_time &&
-		    cluster_quorate_monotime < node->daemon_rem_time) {
+		    cluster_quorate_monotime <= node->daemon_rem_time) {
 			if (!node->killed) {
 				if (cluster_two_node) {
 					/*

2.6.6

Hi Eric, thanks for looking at this, it's a notoriously difficult situation to sort out. I'm not sure we have the same understanding of how the behavior will change with your patch, so let's look at an example, and please let me know if you think these examples don't match what you see (it's been quite a while since I actually tested this).

T = time in seconds, A,B,C = cluster nodes.

At T=1 A,B,C become members and have quorum.
At T=10 a partition creates A,B | C.
At T=11 it merges and creates A,B,C.

At T=12, A,B will have:
cluster_quorate=1
cluster_quorate_monotime=1
C->daemon_rem_time=10

At T=12, C will have:
cluster_quorate=1
cluster_quorate_monotime=11
A->daemon_rem_time=10
B->daemon_rem_time=10

Result:

A,B will kick C from the cluster because cluster_quorate_monotime (1) < C->daemon_rem_time (10), which is what we want.

C will not kick A,B from the cluster because cluster_quorate_monotime (11) > A->daemon_rem_time (10), which is what we want.

It's the simpler case, but does that sound right so far?

...

If the partition and merge occur within the same second, then:

At T=1 A,B,C become members and get quorum.
At T=10 a partition creates A,B | C.
At T=10 it merges and creates A,B,C.

At T=12, A,B will have:
cluster_quorate=1
cluster_quorate_monotime=1
C->daemon_rem_time=10

At T=12, C will have:
cluster_quorate=1
cluster_quorate_monotime=10
A->daemon_rem_time=10
B->daemon_rem_time=10

Result:

A,B will kick C from the cluster because cluster_quorate_monotime (1) < C->daemon_rem_time (10), which is what we want.

C will not kick A,B from the cluster because cluster_quorate_monotime (10) = A->daemon_rem_time (10), which is what we want.

If that's correct, there doesn't seem to be problem so far. If we apply your patch, won't it allow C to kick A,B from the cluster since cluster_quorate_monotime = A->daemon_rem_time?

...

If you're looking at a cluster with an equal partition, e.g. A,B | C,D, then it becomes messy because cluster_quorate_monotime = daemon_rem_time everywhere after the merge. In this case, no nodes will kick others from the cluster, but with your patch, each side will kick the other side from the cluster. Neither option is good. In the past we decided to let the cluster sit in this state so an admin could choose which nodes to remove. Do you prefer the alternative of kicking nodes in this case (with somewhat unpredictable results)? If so, we could make that an optional setting, but we'd want to keep the existing behavior for non-even partitions in the example above.

Hi David,

Thanks very very much for explaining this to me in such nice way;-)

Sure;-)

Aha, yes! Also, this patch doesn't make any sense because it doesn't work if cluster_quorate_monotime (the time when network re-connected) > A->daemon_rem_time (when network disconnected) when there are 3 or more partitions that merge.

Gotcha, thanks! But could you please elaborate a bit more on the meaning of "with somewhat unpredictable results"? IMHO, you mean some inconsistent problems may happen? or just worry about all the service would be interrupted because all nodes would be fenced?

Actually, the reason why I'm working on this problem is because the patch for this issue has been merged into pacemaker:

        [1] https://github.com/ClusterLabs/pacemaker/pull/839

Personally, I think it's a totally bad fix which makes careful thoughts and efforts of dlm_controld in vain, because it make pacemaker fence this node whenever dlm resource agent notices there's fencing going on by "dlm_tool ls | grep -q "wait fencing"", right?

This fix is for:

        [2] https://bugzilla.redhat.com/show_bug.cgi?id=1268313

We've seen unnecessary fence happens in two-node cluster with this fix. E.g. both of two nodes will be fenced when we kill corosync on one of nodes.

Thanks for your suggestion. So far, it looks like an optional setting is much better than this fix. I'd like to have a try;-)

Thanks,
Eric

If both sides of the merged partition are kicking the other out of the cluster at the same time, it's hard to predict which nodes will remain (and it could be none). To resolve an even partition merge, you need to remove/restart the nodes on one of the former halves, i.e. either A,B or C,D. I never thought of a way to do that automatically in this code (maybe higher level code would have more options to resolve it.)

"wait fencing" is normal. Reading those links, it appears the real issue was not identified before the patch was applied.

Remove the bad fix and it should work better.

Two node clusters are a special case of an even partition merge; I'm sure you've seen the lengthy comment about that. In a 2|2 partition merge, we don't kick any nodes from the cluster, as explained above, and it generally requires manual resolution.

But a 1|1 partition merge can sometimes be resolved automatically. Quorum can be disabled in a two node cluster, and the fencing system allowed to race between two partitioned nodes to select a survivor (there are caveats with that.) The area of code you've been looking at (with the long comment) uses the result of the fencing race to resolve the possible partition merge by kicking out the node that was fenced.

Dave

Hi David,

Thanks! Hum, according to the long comments, you've handled the 2/2 even split by way of the low nodeid killing statefull merged numbers. we can do that because the "clean nodes" == "stateful nodes", right? I guess you're saying the case that there're 3 or more partitions that merge, and none could see enough clean nodes?

Yes, agree. But pacemaker guys may complain there's not enough info for them to judge where DLM is. Now DLM outputs log message like: "fence work wait to clear merge $merge_count clean $clean_count part $part_count gone $gone_count". I'm wondering if we can provide these info and how long DLM has been stuck by "dlm_tool $some_cmd"?

Also, I'm working an option (like enable_force_kick) as you suggested;-)

Yes, will try to persuade pacemaker to drop that patch;-)

Yes, I can understand this;-)

Thanks a lot!
Eric

Interesting, I'd forgotten about that bit of code, so I was wrong to say that we do nothing after a 2/2 partition merge.

That sounds about right, this is a fairly narrow special case that mainly helps in the case of two evenly split partitions. There could be some cases with more nodes and 3+ partitions where it might help.

Yes, that's a good idea. dlm_tool should clearly report if the dlm is blocked because of a stateful partition merge. Perhaps a new global variable printed by 'dlm_tool status -v'?

I'm not exactly sure of the condition when we'd set this new variable, could you try it out? For a start, maybe something similar to this:

static int stateful_merge_wait;
...

        if (!cluster_two_node && merge_count) {
                log_retry(retry_fencing, "fence work wait to clear merge %d clean %d part %d gone %d",
                          merge_count, clean_count, part_count, gone_count);

                if ((clean_count >= merge_count) && !part_count && (low == our_nodeid))
                        kick_stateful_merge_members();

+ if ((clean_count < merge_count) && !part_count)
+ stateful_merge_wait=1;

                retry = 1;
                goto out;
        }

(and added to print_state_daemon)

Dave