[Cluster-devel] [PATCH] dlm_controld.pcmk: Fix membership change judging issue

[Andrew Beekhof]

On Thu, May 13, 2010 at 11:51 AM, Lars Marowsky-Bree <lmb at novell.com> wrote:
> On 2010-05-13T16:49:27, Jiaju Zhang <jjzhang.linux at gmail.com> wrote:
>
> Hi Jiaju,
>
> thanks for the patch and the technical explanation!
>
>> Because all the components get the membership info eventually from
>> Corosync, IMO, for dlm_controld.pcmk, there is no need to wait
>> Pacemaker/crmd to finish all the information processing related to
>> membership change.
>>
>> Patched attached below, any review and comments are highly
>> appreciated!
>
> I'd only like to add the user-visible impact of this - namely, when a
> previously unknown node joins a pcmk cluster (with running DLM etc),
> nodes will hang.
>
> So it is fairly important to get fixed.
>
>
> So, with my limited knowledge of the code, I think the fix is sound. I
> have just one clerical comment:
>
>>  void process_cluster(int ci)
>>  {
>> -    ais_dispatch(ais_fd_async, NULL);
>>      update_cluster();
>>  }
>
> Can this function be removed?

No, it can't.
Remove that and the membership (and crm_peer_id_cache) stops being updated.

Which makes me very suspicious of the whole patch because it clearly
wasn't tested very well.

> It doesn't do anything any more. Or is
> there something that should be done with the "ci" argument?
>
>
> Regards,
>    Lars
>
> --
> Architect Storage/HA, OPS Engineering, Novell, Inc.
> SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
> "Experience is the name everyone gives to their mistakes." -- Oscar Wilde
>
>