Skip to content

[database] Fix docker-database flush_unused_database failed issue#11600

Merged
liuh-80 merged 1 commit intosonic-net:masterfrom
liuh-80:dev/liuh/fix-flush-unused-db
Aug 3, 2022
Merged

[database] Fix docker-database flush_unused_database failed issue#11600
liuh-80 merged 1 commit intosonic-net:masterfrom
liuh-80:dev/liuh/fix-flush-unused-db

Conversation

@liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Aug 2, 2022 •
edited
Loading

Why I did it

Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

How I did it

Change flush_unused_database code to use swsscommon API:
Change get_instancelist to getInstanceList.
Change get_dblist to getDbList.

How to verify it

Pass all E2E test.
Manually check syslog make sure error log not exist and swss, syncd, bgp service started.
Search code in Azure make sure there all similer case are fixed in this PR.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 2, 2022

Will validate service and syslog after build VS image finished.

@liuh-80 liuh-80 mentioned this pull request Aug 2, 2022
Closed
@liuh-80 liuh-80 marked this pull request as ready for review August 2, 2022 08:51
@liuh-80 liuh-80 requested a review from lguohan as a code owner August 2, 2022 08:51
@liuh-80 liuh-80 requested review from qiluo-msft and vaibhavhd August 2, 2022 08:52
@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 2, 2022

Manually verify the script fixed by following command:

root@vlab-01:/# /usr/local/bin/flush_unused_database
root@vlab-01:/#

VS image download from pipeline:
https://artprodcus3.artifacts.visualstudio.com/Af91412a5-a906-4990-9d7c-f697b81fc04d/be1b070f-be15-4154-aade-b1d3bfb17054/_apis/artifact/cGlwZWxpbmVhcnRpZmFjdDovL21zc29uaWMvcHJvamVjdElkL2JlMWIwNzBmLWJlMTUtNDE1NC1hYWRlLWIxZDNiZmIxNzA1NC9idWlsZElkLzEyOTY2Ny9hcnRpZmFjdE5hbWUvc29uaWMtYnVpbGRpbWFnZS52cw2/content?format=file&subPath=%2Ftarget%2Fsonic-vs.img.gz

@vaibhavhd
Copy link
Contributor

vaibhavhd commented Aug 3, 2022

LGTM, it would be ideal if we could add a unit test for this to catch this issue on PR tests.

qiluo-msft
qiluo-msft reviewed Aug 3, 2022
View reviewed changes
dockers/docker-database/flush_unused_database
for instname, v in instlists.items():
insthost = v['hostname']
instsocket = v['unix_socket_path']
insthost = v.hostname
Copy link
Collaborator

@qiluo-msft qiluo-msft Aug 3, 2022

Choose a reason for hiding this comment

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

hostname

How easy if you fix on the SWIG side and keep app code unchanged?

Copy link
Contributor Author

@liuh-80 liuh-80 Aug 3, 2022

Choose a reason for hiding this comment

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

That need make the RedisInstInfo become a dictionary, need about 40 lines code change, I make this change before and reverted:
sonic-net/sonic-swss-common@6a4a7b0

I can make that change if necessary.

Copy link
Collaborator

@qiluo-msft qiluo-msft Aug 3, 2022

Choose a reason for hiding this comment

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

Not necessary then.

@liuh-80 liuh-80 merged commit 45ded68 into sonic-net:master Aug 3, 2022
@yxieca
Copy link
Contributor

yxieca commented Aug 8, 2022

@liuh-80 please raise separate PR fro 202205 branch

@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 10, 2022

@liuh-80 please raise separate PR fro 202205 branch
ACK, will submit PR later.

liuh-80 added a commit that referenced this pull request Aug 10, 2022
@liuh-80
Fix docker database flush_unused_database failed issue (#11600)
9106980 
Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

Change flush_unused_database code to use swsscommon API:
Change get_instancelist to getInstanceList.
Change get_dblist to getDbList.

Pass all E2E test.
Manually check syslog make sure error log not exist and swss, syncd, bgp service started.
Search code in Azure make sure there all similer case are fixed in this PR.

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205

Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net>
@liuh-80 liuh-80 mentioned this pull request Aug 10, 2022
Merged
14 tasks
@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 10, 2022

@liuh-80 please raise separate PR fro 202205 branch
ACK, will submit PR later.

CHerry-pick for 202205 created here: #11677

liuh-80 added a commit that referenced this pull request Aug 12, 2022
@liuh-80
Fix docker database flush_unused_database failed issue (#11600) (#11677)
7e4883e 
Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

Change flush_unused_database code to use swsscommon API:
Change get_instancelist to getInstanceList.
Change get_dblist to getDbList.

Pass all E2E test.
Manually check syslog make sure error log not exist and swss, syncd, bgp service started.
Search code in Azure make sure there all similer case are fixed in this PR.

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205

Fix docker-database flush_unused_database failed issue: #11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net>

<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it

#### How I did it

#### How to verify it

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
skbarista pushed a commit to skbarista/sonic-buildimage that referenced this pull request Aug 17, 2022
@liuh-80 @skbarista
Fix docker database flush_unused_database failed issue (sonic-net#11600…
7a40560 
…) (sonic-net#11677)

Fix docker-database flush_unused_database failed issue: sonic-net#11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

Change flush_unused_database code to use swsscommon API:
Change get_instancelist to getInstanceList.
Change get_dblist to getDbList.

Pass all E2E test.
Manually check syslog make sure error log not exist and swss, syncd, bgp service started.
Search code in Azure make sure there all similer case are fixed in this PR.

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205

Fix docker-database flush_unused_database failed issue: sonic-net#11597
When change flush_unused_database from use swsssdk to use swsscommon, get_instancelist() and get_dblist() name changed but not update.

<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net>

<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it

#### How I did it

#### How to verify it

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
@anamehra anamehra mentioned this pull request Sep 12, 2022
Closed
@rlhui
Copy link
Contributor

rlhui commented Sep 14, 2022

@qiluo-msft - should this PR be reverted in 202205, as it caused issue mentioned?

liuh-80 added a commit that referenced this pull request Sep 15, 2022
@liuh-80
Revert "Fix docker database flush_unused_database failed issue (#11600)…
c8ccfea 
… (#11677)"

This reverts commit 7e4883e.
@liuh-80 liuh-80 mentioned this pull request Sep 15, 2022
Merged
7 tasks
@liuh-80
Copy link
Contributor Author

liuh-80 commented Sep 15, 2022

Remove cherry-pick labels from this PR, because all dependency changes are not included in 202205 branch.
cherry-pick cause code error and will revert with #12084

@liuh-80 liuh-80 deleted the dev/liuh/fix-flush-unused-db branch September 15, 2022 06:17
gechiang pushed a commit that referenced this pull request Sep 15, 2022
@liuh-80
Revert "Fix docker database flush_unused_database failed issue (#11600)…
e51368d 
… (#11677)" (#12084)

This reverts commit 7e4883e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

@vaibhavhd vaibhavhd Awaiting requested review from vaibhavhd

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@liuh-80 @vaibhavhd @yxieca @rlhui @qiluo-msft