pacemaker icon indicating copy to clipboard operation
pacemaker copied to clipboard

Published 20 hours ago • verified publisher icon ClusterLabs

(WIP) Make controller go through attribute manager to clear transient attributes.

Open HideoYamauchi opened this issue 6 years ago • 8 comments

Hi All,

This is for discussion only. I have confirmed the rough behavior, but the source has not been completed yet. I think there are many missing fixes.

This PR is based on the following Ken's PR as a base.

  • https://github.com/ClusterLabs/pacemaker/pull/1695

I made the following changes as the first realization method.

It is unknown whether it can cope with clusters with many nodes or when there are many attributes. In my modification, since clearing of attributes is handled in smaller units, improvement may be necessary.

  • Mark when receiving the ATTRD_OP_PEER_CLEAR message. In order to cope with crmd restarting, we will not immediately delete the attribute. For remote node attributes, delete them immediately. Deletes the attribute when the mark of the request to delete the attribute from crmd and the event of the node leaving attrd are collected.(It should be noted that the order in which this mark and the event occur may be reversed.) With this fix, attributes are always deleted via the ATTRD_OP_PEER_CLEAR message. Although this fix alone does not solve it, it seems to be effective even if only attrd is restarted in the future.

  • Add ATTRD_OP_ATTR_REMOVE as a new message. After completing the update of cib, the Writer is delete the memory attribute of attrd. At this time, send ATTRD_OP_ATTR_REMOVE to the attrd member of the cluster. Other attrds that received ATTRD_OP_ATTR_REMOVE also delete the memory attributes. In order to reliably synchronize the attributes in the cluster, we have made such a mechanism.

  • It does not incorporate the correspondence of the old CRM_ATTR_PROTOCOL version.

  • When the attrd leaving the cluster once again participates, the processing flag is cleared. It is when attrd received the CRM_ATTR_PROTOCOL message Actually, you should clear the processing flag when crmd comes in, but attrd can not detect it at the moment.

  • Forced write decision by force_write has been moved to write_attributes () write_attribute (). This is because writing from write_attribute () may occur....

  • For now it seems to be difficult to delete the attribute of an isolated attrd if crmd does not restart.

  • etc..

This PR has already found the next problem.

  • The flag is cleared incompletely, after crmd restarts, the attribute is deleted when attrd restarts.(Need something good way)
  • ATTRD_OP_PEER_CLEAR will not be sent from the crmd of the new DC node after the cluster without STONITH is broken. To @kgaillot : Correction to send ATTRD_OP_PEER_CLEAR message from new DC node is necessary. Do you know where you need to make corrections?

The problem of attrd remaining after this correspondence.

  • However, if Election is delayed, problems remain. (https://bugs.clusterlabs.org/show_bug.cgi?id=5358#c21). Currently, setting the "transition-delay" only prevents the problem.

  • Unlike this problem, we must also take into consideration that the attribute will be lost if only attrd is restarted in the future. The current attrd deletes the attribute when it starts up. Even if we exchange sync_response messages with other nodes, we reset them with NULL. As a result, when attrd restarts, its attributes disappear. Especially when configuring a cluster with a single node, the attribute is completely lost. For example, when restarting attrd, it may be effective not to erase the attribute by pacemakerd passing the parameter.

Best Regards, Hideo Yamauchi.

HideoYamauchi avatar Feb 21 '19 23:02 HideoYamauchi

IMO,I have not confirmed the correction method, but I think that it is better for Writer to erase it in a stroke in the same way as it did with erase_status_tag ().

HideoYamauchi avatar Feb 28 '19 23:02 HideoYamauchi

@HideoYamauchi , @kgaillot may we merge this PR, or at least the https://github.com/ClusterLabs/pacemaker/pull/1693 ?

aleksei-burlakov avatar Feb 12 '20 16:02 aleksei-burlakov

Hi aleksei,

This fix is fairly old and experimental and not completed. Therefore, this fix cannot be merged.

I think we need to reconsider again.

Best Regards, Hideo Yamauchi.

HideoYamauchi avatar Feb 13 '20 00:02 HideoYamauchi

Hi aleksei,

This fix is fairly old and experimental and not completed. Therefore, this fix cannot be merged.

I think we need to reconsider again.

Best Regards, Hideo Yamauchi.

@HideoYamauchi , @kgaillot what if we merge the #1693 ? It should not make any regression.

aleksei-burlakov avatar Feb 13 '20 08:02 aleksei-burlakov

@HideoYamauchi , @kgaillot what if we merge the #1693 ? It should not make any regression.

Unfortunately it would reintroduce a crash at scale. For the time being it's a choice of lesser evils, normal operation of pacemaker at scale being more important than corner cases when daemons respawn.

This is a tricky problem. Moving the functionality to attrd makes sense and avoids issues with the controller respawning, and should have fewer corner cases, but we do have to watch out for the corner cases that do remain, and we have to handle backward compatibility in mixed-version clusters where some nodes have the new feature and some don't.

kgaillot avatar Feb 13 '20 15:02 kgaillot

Hi Ken,

If there is a lot of demand for improvement of this problem, I think it is good to have the opportunity to tackle this problem again somewhere.

Best Regards, Hideo Yamauchi.

HideoYamauchi avatar Feb 13 '20 23:02 HideoYamauchi

I would like to take over this PR in the #2020. @HideoYamauchi , I would appreciate your comments or any help with it=)

aleksei-burlakov avatar Mar 24 '20 08:03 aleksei-burlakov

Hi aleksei,

I would like to take over this PR in the #2020. @HideoYamauchi , I would appreciate your comments or any help with it=)

Okay! If you need my comments, please contact me.

Best Regards, Hideo Yamauchi.

HideoYamauchi avatar Mar 26 '20 02:03 HideoYamauchi