Now that we don't fall through we don't need this check or the similar one below

oops

Actually (thankfully) this shouldn't be necessary. AC_CHECK_FUNCS() will already define that, so we were just unnecessarily defining the alternate name. We can just drop the second argument (i.e. the AC_DEFINE) altogether.

Then let's make it consistent if we are sure it works the same on all platforms/versions we do support.

In public clouds, nowadays it happens more often than before that sub-daemons are unresponsive to IPC and get respawned.

As we know, if it's controller that respawns, the node will lose all its transient attributes in the CIB status without being written again. Not only the resources that rely on the attributes will get impacted, but also missing of the internal attribute #feature-set will result into confusing MIXED-VERSION condition being shown from interfaces like crm_mon.

So far PCMK_fail_fast=yes probably is the only workaround to get the situation back into sanity but of course at a cost of node reboot.

While we've been trying to address it with the idea like:
#1699

, I'm not sure if it'd make sense at all to increase the tolerance here such as PCMK_PROCESS_CHECK_RETRIES or make it configurable... Otherwise should we say that 5 failures in a row are anyway bad enough to trigger a recovery?

Sry I may be missing the reason for your comment here.
Previously IPC wasn't checked on a periodic basis for all subdaemons.
Numbers are kind of arbitrary. 1s is kind of a lower limit that makes sense for retries. Failing after 5 retries was the attempt to make it as reactive as before for cases where IPC was checked before already.

Nothing is wrong with the changes in this PR. Just for bringing up the topic in the context here :-)

Coincidentally I recently created https://projects.clusterlabs.org/T950 regarding this code, but it's not related unless you've only seen issues at cluster shutdown.

https://projects.clusterlabs.org/T73 is not directly related either but could affect the timing.

There is a 1s delay between checks of all subdaemons, so if they're all up, that's at least 6s between checks for any one subdaemon. 5 tries (30s) does seem plenty of time, so I wouldn't want to raise that. If a cloud host can't get enough cycles in 30s to respond to a check, it's probably unsuitable as an HA node.

Thanks for the info and opinion. I agree.

@wenningerk Any idea about how pacemaker-based processes the ping request from other sub-daemons?
Thanks.

Don't know/remember in that detail - so give me a bit to check. Maybe sbdy knowing the answers from the top of his/her head can jump in.

AFAICS, despite how a system runs into a difficult situation, the situation may be reflected by two factors that can affect pacemakerd's liveness checks on sub-daemons:

1. How long the situation lasts
2. How difficult the situation is

Regarding factor 1), as also mentioned in above, there's at least 6s between checks for a specific sub-daemon. So 5 tries (30s) seem to offer a certain degree of tolerance.

While regarding factor 2) for a single liveness check, it surely affects how soon pacemakerd can receive a response from a sub-daemon. So far it's restricted by the 2s poll timeout though:

It seems a little too sensitive in practice. We've seen users' cases where it often timed out right before the sub-daemon was about to send out a response.

And such timeouts of liveness checks are kind of premature IMO. They almost always appear when there are no real timeouts/failures occurring on the production IPCs between pacemaker daemons yet. AFAICS for the production IPCs, timeouts are usually longer than 5s, for example in the typical cases with crm_ipc_send().

I wonder whether it'd make sense to increase the tolerance on this specific IPC for liveness check. There have been some experiments in users' clusters where an enlarged poll timeout definitely increases the tolerance. The question is whether it'd make sense to generally enlarge the hardcoded poll timeout or even make it configurable.

I would prefer to not make this configurable. We already have so many tuneables that it can be pretty hard to keep track of it all.

I would be okay with increasing this timeout, because I've also seen the "Subdaemon is unresponsive to IPC" message more than I'd like, when what's really happening is the daemon is just bogged down dealing with too many messages. In general, this whole area seems more problematic than I wish it were. Daemons get bogged down too easily, and we give up on them too easily. As the number of resources increases, the problems just get worse.

On the other hand, this is called by child_liveness which in turn is called in a loop by find_and_track_existing_processes. I worry that if we increase the timeout here, one slow daemon is going to have ripple effects for other daemons later on in the loop. Ken had mentioned at one point making this truly async, which I think would be a good thing to do along with increasing the timeout, but obviously that is going to be a more difficult patch.

Finally, I've been looking at this code a little bit recently, and our process management code is pretty gross. It'd be great if someone with a really good understanding of this stuff could clean it up, suggest a library we could be using, etc.

Not saying relaxing the timeout by a bit wouldn't make sense but there is actually an idea behind making this trigger a little bit earlier than actual ipc-timeouts. Like this we can see how often we're running in a situation where it is getting close to ipc timing out. And actions are anyway just taken if that happens 5 times in a row.