Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MultiDB] use sonic-db-cli PING and fix wrong multiDB API in NAT #4541

Merged
merged 1 commit into from
May 6, 2020
Merged

[MultiDB] use sonic-db-cli PING and fix wrong multiDB API in NAT #4541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion dockers/docker-database/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ RUN apt-get clean -y && \

COPY ["supervisord.conf.j2", "/usr/share/sonic/templates/"]
COPY ["docker-database-init.sh", "/usr/local/bin/"]
COPY ["ping_pong_db_insts", "/usr/local/bin/"]
COPY ["database_config.json", "/etc/default/sonic-db/"]
COPY ["files/supervisor-proc-exit-listener", "/usr/bin"]
COPY ["critical_processes", "/etc/supervisor"]
Expand Down
40 changes: 0 additions & 40 deletions dockers/docker-database/ping_pong_db_insts
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions dockers/docker-nat/restore_nat_entries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
IP_PROTO_TCP = '6'

MATCH_CONNTRACK_ENTRY = '^(\w+)\s+(\d+).*src=([\d.]+)\s+dst=([\d.]+)\s+sport=(\d+)\s+dport=(\d+).*src=([\d.]+)\s+dst=([\d.]+)\s+sport=(\d+)\s+dport=(\d+)'
REDIS_SOCK = "/var/run/redis/redis.sock"

logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)
Expand All @@ -44,7 +43,7 @@ def add_nat_conntrack_entry_in_kernel(ipproto, srcip, dstip, srcport, dstport, n

# Set the statedb "NAT_RESTORE_TABLE|Flags", so natsyncd can start reconciliation
def set_statedb_nat_restore_done():
statedb = swsscommon.DBConnector(swsscommon.STATE_DB, REDIS_SOCK, 0)
statedb = swsscommon.DBConnector("STATE_DB", 0)
tbl = swsscommon.Table(statedb, "NAT_RESTORE_TABLE")
fvs = swsscommon.FieldValuePairs([("restored", "true")])
tbl.set("Flags", fvs)
Expand Down
6 changes: 5 additions & 1 deletion files/build_templates/docker_image_ctl.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ function postStartAction()
link_namespace $DEV
fi
# Wait until redis starts
/usr/bin/docker exec database$DEV ping_pong_db_insts
# TODO: should use $SONIC_DB_CLI if Judy's PR 4477 is in first, otherwise PR 4477 should change this part
until [[ $(/usr/bin/sonic-netns-exec "$NET_NS" sonic-db-cli PING | grep -c PONG) -gt 0 ]]; do
sleep 1;
done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@judyjoseph Could you please experiment with this part? It supposed to fix the corrupted config file issue. I wish your PR 4477 merge after this one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@judyjoseph Could you please experiment with this part? It supposed to fix the corrupted config file issue. I wish your PR 4477 merge after this one.

the corrupted config file issue will be fixed after the swsssdk submodule updated plus this PR
Advance sonic-py-swsssdk pointer #4496

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this will fix the issue I saw earlier. The issue I saw was that the config json files was present but empty as the database_config.json and database_global.json files are generated using "j2".

So now again sonic-db-cli PING, we will indirectly try to open the database_config.json/database_global.json file which could be in same state of "present but don't have contents" and json.load will fail.

I believe the solution would be some sort of synchronization between where we generate the json files ( which is in the database docker ) .. and the other scripts ..were we use to check with PING-PONG.

Copy link
Collaborator Author

@dzhangalibaba dzhangalibaba May 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this will fix the issue I saw earlier. The issue I saw was that the config json files was present but empty as the database_config.json and database_global.json files are generated using "j2".

So now again sonic-db-cli PING, we will indirectly try to open the database_config.json/database_global.json file which could be in same state of "present but don't have contents" and json.load will fail.

I believe the solution would be some sort of synchronization between where we generate the json files ( which is in the database docker ) .. and the other scripts ..were we use to check with PING-PONG.

IF open file failed or file context is not correct , sonic-db-cli PING won't return 'PONG' and the until loop continue to try. This behavior will be there after swsssdk submodule updated to latest

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this logic with the sonic-db-cli PONG works correctly even though there are certain json load errors initially.

Btw I find there is no dependency on sonic-py-swsssdk pointer #4496. It works ok with the current PING/PONG implementation in the swsssdk submodule in sonic-buildimage. Please confirm if that is the case ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this logic with the sonic-db-cli PONG works correctly even though there are certain json load errors initially.

Btw I find there is no dependency on sonic-py-swsssdk pointer #4496. It works ok with the current PING/PONG implementation in the swsssdk submodule in sonic-buildimage. Please confirm if that is the case ?

The logic is working, if something bad happened when loading json file, it not returning 'PONG' and keep trying in the loop. From functionality point of view, it does not depend on 4496. But there is a exception internally, after 4496 is merged , the exception is handled as a failure case. This is the only difference. So we can merge this PR for now , there is no functionality effect and will work as you said

if [[ ("$BOOT_TYPE" == "warm" || "$BOOT_TYPE" == "fastfast") && -f $WARM_DIR/dump.rdb ]]; then
rm -f $WARM_DIR/dump.rdb
else
Expand Down
4 changes: 3 additions & 1 deletion files/image_config/warmboot-finalizer/finalize-warmboot.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ function wait_for_database_service()
debug "Wait for database to become ready..."

# Wait for redis server start before database clean
/usr/bin/docker exec database ping_pong_db_insts
until [[ $(sonic-db-cli PING | grep -c PONG) -gt 0 ]]; do
sleep 1;
done

# Wait for configDB initialization
until [[ $(sonic-db-cli CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
Expand Down
2 changes: 1 addition & 1 deletion files/scripts/configdb-load.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env bash

# Wait until redis starts
until [[ $(redis-cli ping | grep -c PONG) -gt 0 ]]; do
until [[ $(sonic-db-cli PING | grep -c PONG) -gt 0 ]]; do
sleep 1;
done

Expand Down
5 changes: 4 additions & 1 deletion files/scripts/swss.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ function validate_restore_count()
function wait_for_database_service()
{
# Wait for redis server start before database clean
/usr/bin/docker exec database$DEV ping_pong_db_insts
# TODO: should use $SONIC_DB_CLI if Judy's PR 4477 is in first, otherwise PR 4477 should change this part
until [[ $(/usr/bin/sonic-netns-exec "$NET_NS" sonic-db-cli PING | grep -c PONG) -gt 0 ]]; do
sleep 1;
done

# Wait for configDB initialization
until [[ $(sonic-netns-exec "$NET_NS" sonic-db-cli CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
Expand Down
5 changes: 4 additions & 1 deletion files/scripts/syncd.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ function check_warm_boot()
function wait_for_database_service()
{
# Wait for redis server start before database clean
/usr/bin/docker exec database$DEV ping_pong_db_insts
# TODO: should use $SONIC_DB_CLI if Judy's PR 4477 is in first, otherwise PR 4477 should change this part
until [[ $(/usr/bin/sonic-netns-exec "$NET_NS" sonic-db-cli PING | grep -c PONG) -gt 0 ]]; do
sleep 1;
done

# Wait for configDB initialization
until [[ $(sonic-netns-exec "$NET_NS" sonic-db-cli CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
Expand Down