Multiple restart of swss during config load fails to start swss

[antony-rheneus]

https://github.com/Azure/sonic-buildimage/blob/67463f18b2ea396c1b3bab87575f803376a8046e/files/build_templates/swss.service.j2#L13

@jleveque, Interval has been increased to from default 10sec to 1200sec, but burst has been decreased from default 5 to 3.
Since burst is too low in longer interval timespan, swss was not started by systemd.
Can we revert the burst/increase it?

[jleveque]

@antony-rheneus: Could you please explain your issue more? The new settings (StartLimitInterval = 1200sec, StartLimitBurst = 3) now mean that if systemd has already restarted swss 3 times in the past 20 minutes, it should stop trying to restart the service and mark it as failed. Under what circumstances are you encountering this? It should never occur under normal operation.

[nikos-github]

@jleveque We are having the same issue. Configuration that isn't incremental yet can't be pushed within the specified number of time limit, is causing swss to not start again.

[jleveque]

@nikos-github: To clarify, you have a need to perform > 3 configuration pushes (and thus >3 SwSS restarts, due to lack of incremental config) within 20 minutes? Is this correct?

[antony-rheneus]

@jleveque , If you run test suite, this fails as test suite does change configs multiple times and test 1by1.
Excerpt " if systemd has already restarted swss 3 times in the past 20 minutes, it should stop trying to restart the service and mark it as failed."
How can you be sure swss restart more than thrice is not vaild scenario, I personally feel this is a valid scenario?

Continuous restart is InValid Only if there is issue in application which exited or core dumped, and then endless loop in the same state has to be avoided. For this we cannot add a prevention in generic infra which affects intentional restarts.

[antony-rheneus]

@jleveque Would you give some pointers why this was changed from default system service values? If you provide me some insights it would be helpful for me to understand the reason for the change, as you would have analysed to come up with these new values

[nikos-github]

@nikos-github: To clarify, you have a need to perform > 3 configuration pushes (and thus >3 SwSS restarts, due to lack of incremental config) within 20 minutes? Is this correct?

@jleveque That is correct. Currently not all configuration pertaining to sonic can be applied incrementally or without restarting swss. Keep in mind that users may also push configuration through our software at different times which when applied will force a swss restart. I don't think there is a deterministic way to predict how many times swss should be allowed to restart and in what interval.

[jleveque]

@antony-rheneus: This was changed from the default values once we added the 'auto-restart-upon-critical-process-crash' feature (#2845). This is to prevent SONiC from indefinitely restarting the service if there is something causing one of the critical processes to crash consistently.

[antony-rheneus]

can we add "systemctl reset-failed swss" to reset the restart counter in the sonic-utilities/ where config load/load_minigraph is being called?
This is to ensure the test suite doesn't report failure for valid multiple restarts

[avi-milner]

can we add "systemctl reset-failed swss" to reset the restart counter in the sonic-utilities/ where config load/load_minigraph is being called?
This is to ensure the test suite doesn't report failure for valid multiple restarts

also for config reload operation will need to clear counter , this is not only for tests suites, but also for allowing pusing/changing new configurations as much as desired without restrictions to it

[jleveque]

can we add "systemctl reset-failed swss" to reset the restart counter in the sonic-utilities/ where config load/load_minigraph is being called?
This is to ensure the test suite doesn't report failure for valid multiple restarts

We can. This is something I was already considering adding. I'll look into creating a PR.

[jleveque]

@antony-rheneus, @avi-milner: PR here: sonic-net/sonic-utilities#607. Please review.

[jleveque]

Should be addressed by sonic-net/sonic-utilities#607

[avi-milner]

hi @jleveque ,
it seems that the fix is not working as we expected,
when we call to config_reload / config_load/ config_load_minigraph
from this code you only reset failed counter for services that are already in failed state, this still causes the config load scenarios to fail after running them within the time window of 20 minutes, 3 times

can you please fix to always reset failed counter for config load commands ?

i have opened #sonic-net/sonic-utilities#616
for this