Warm reboot: Don't load json conifg like copp, ininip, ports and switch again upon… #1939
Conversation
… swss warm start Signed-off-by: Jipan Yang <[email protected]>
| if [ "$WARM_START" == "true" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
There was a problem hiding this comment.
what if we have modified any of the json?
There was a problem hiding this comment.
This is a good question. At restore phase, you get whatever the previous configurations are.
Should anything be changed for any configuration in the json files, probably there should be specific script, or program implemented for the delta update. It is similar to configDB data change.
I doubt this script is the right place to deal with each possible change. Maybe the long term solution is to eliminate these json files and make them part of the configDB.
|
retest this please |
|
Related to issues: |
…red at system reboot Signed-off-by: Jipan Yang <[email protected]>
jipanyang
changed the title
| exit 0 | ||
| fi | ||
|
|
||
| WARM_START=`redis-cli -n 4 hget "WARM_RESTART|swss" enable` |
There was a problem hiding this comment.
WARM_RESTART|swss [](start = 33, length = 17)
In the scope of this PR, what is the use case for
- WARM_RESTART|system == false
- WARM_RESTART|swss == true
Intuitively, this module should care only the swss one.
There was a problem hiding this comment.
There are two scenarios:
<1> Individual SWSS docker restart.
either system == true or swss == true, warm start will be performed.
<2> system restart.
It is to be supported. But we may hit issue when system == false, swss == true. data would have been flushed at system init. SWSS docker has to perform extra check to make sure data exits for state restore.
| if [ "$WARM_START" == "true" ]; then | ||
| # We have to make sure db data has not been flushed. | ||
| WARM_START=`redis-cli -n 6 hget "WARM_RESTART_TABLE|orchagent" restart_count` | ||
| if [ -n "$WARM_START" ] && [ "$WARM_START" != "0" ]; then |
There was a problem hiding this comment.
[](start = 2, length = 2)
This indentation is not necessary.
There was a problem hiding this comment.
ok. will update it
| WARM_START=`redis-cli -n 6 hget "WARM_RESTART_TABLE|orchagent" restart_count` | ||
| if [ -n "$WARM_START" ] && [ "$WARM_START" != "0" ]; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
fi [](start = 4, length = 2)
else, should we treat it as 'logic error' and return error code immediately? Your code fallbacks to cold start, which may hide bugs in other components. #Closed
There was a problem hiding this comment.
This is to handle the case of system cold boot while swss docker warm enabled.
swss docker warm restart knob is only supposed to handle the scenario of individual docker warm restart. #Resolved
| WARM_START=`redis-cli -n 4 hget "WARM_RESTART|swss" enable` | ||
| if [ "$WARM_START" == "true" ]; then | ||
| # We have to make sure db data has not been flushed. | ||
| WARM_START=`redis-cli -n 6 hget "WARM_RESTART_TABLE|orchagent" restart_count` |
There was a problem hiding this comment.
WARM_START [](start = 2, length = 10)
Don't reuse variable names, especially in embedded scopes.
…at update Signed-off-by: Jipan Yang <[email protected]>
… swss warm start
Signed-off-by: Jipan Yang [email protected]
- What I did
Some of the configurations in copp, ininip, ports and switch are create only, they should be applied once only.
For swss warm start, same configurations already exist in appDB and have been restored.
- How I did it
Skip swss json load upon warm start.
- How to verify it
cold start,
enable warm start, start again.
- Description for the changelog