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.

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

Conversation

dzhangalibaba
Copy link
Collaborator

@dzhangalibaba dzhangalibaba commented May 6, 2020

  • use sonic-db-cli PING instead of earlier ping_pong_db_insts script, changed back to until loop format in shell
  • fix nat related multiDB API issue

Signed-off-by: Dong Zhang d.zhang@alibaba-inc.com

@dzhangalibaba
Copy link
Collaborator Author

@qiluo-msft

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

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

@qiluo-msft qiluo-msft requested a review from judyjoseph May 6, 2020 18:45
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants