fdbmonitor restart-delay unclear documentation

[johscheuer]

fdbmonitor has a field restart-delay. Based on the documentation: The maximum number of seconds (subject to jitter) that fdbmonitor will delay before restarting a failed process.. I would assume that failed process means a process that exits with an exit code other than 0. But the actual implementation doesn't look at the exit code: https://github.com/apple/foundationdb/blob/main/fdbmonitor/fdbmonitor.h#L395-L405 + https://github.com/apple/foundationdb/blob/main/fdbmonitor/fdbmonitor.cpp#L646. So we either have to correct the documentation or fix the behaviour in fdbmonitor.

I saw this when I looked into #11764.

[johscheuer]

CC @spraza

[johscheuer]

I don't have a strong opinion on either but I think using the exit code and only delaying processes that have an exit code different than 0 should be delayed. My understanding of this setting is to prevent a process to crash loop directly in a short amount of time.

[johscheuer]

For completeness: In the fdb-kubernetes-monitor we are only delaying the restart when the process exit code was not 0: https://github.com/apple/foundationdb/blob/main/fdbkubernetesmonitor/monitor.go#L453-L459

[spraza]

Looked at the code. You're right, currently the delay calculation doesn't account for exit code. I think it's reasonable to only have delays for failing (non zero) exited processes. Testing the fix would probably take more time than the fix itself. I can send the PR out but it may take a few days before I get to it. Happy to review the PR if one of you is interested to fix this (@johscheuer, @mpatou-openai).