[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Fri, May 14, 2010 at 12:25 AM, Andrew Beekhof <andrew beekhof net> wrote:
> On Thu, May 13, 2010 at 11:51 AM, Lars Marowsky-Bree <lmb novell com> wrote:
>> On 2010-05-13T16:49:27, Jiaju Zhang <jjzhang linux 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.

Agreed.

Both update_cluster and process_cluster can't be removed. Yes, moving
the place of ais_dispatch should have nothing to do with this issue.
Oh, this is because I added it in the debugging stage and forgot to
remove it in the final version, just sending the working version to
the list, sorry, I'll posted an updated patch soon ;-)

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

I have tested in my environment and it works for me.

Thanks,
Jiaju


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]