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

Jump to bottom

Re-write sonic-db-cli with c++ for sonic startup performance issue #607

Merged
merged 21 commits into from
May 26, 2022
Merged

Re-write sonic-db-cli with c++ for sonic startup performance issue #607

merged 21 commits into from
May 26, 2022

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Apr 24, 2022 •
edited
Loading

Why I did it

Fix sonic-db-cli high CPU usage on SONiC startup issue: https://github.com/Azure/sonic-buildimage/issues/10218
ETA of this issue will be 2022/05/31

How I did it

Re-write sonic-cli with c++.

How to verify it

Add c++ unit test to cover all code.
Pass all E2E test scenario.

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

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

Description for the changelog

Re-write sonic-cli with c++ for sonic startup performance issue

Link to config_db schema for YANG module changes

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

@liuh-80 liuh-80 mentioned this pull request Apr 24, 2022
Closed
6 tasks
liuh-80
liuh-80 commented Apr 24, 2022
View reviewed changes
sonic-db-cli/tests/main.cpp Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed May 12, 2022
View reviewed changes
tests/Makefile.am Outdated Show resolved Hide resolved
qiluo-msft
qiluo-msft reviewed May 24, 2022
View reviewed changes
common/redisreply.h Outdated
private:
void checkStatus(const char *status);
void checkReply();
std::string to_string(redisReply *reply);
Copy link
Contributor

@qiluo-msft qiluo-msft May 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

to_string

This function could be:

  1. static member function
  2. public function
  3. or global function, since it will be useful to any redisReply. #Closed

qiluo-msft
qiluo-msft reviewed May 24, 2022
View reviewed changes
common/redisreply.cpp
@@ -158,7 +159,7 @@ void RedisReply::checkReplyType(int expectedType)
const char *err = (m_reply->type == REDIS_REPLY_STRING || m_reply->type == REDIS_REPLY_ERROR) ?
m_reply->str : "NON-STRING-REPLY";

string errmsg = "Expected to get redis type " + to_string(expectedType) + " got type " + to_string(m_reply->type) + ", err: " + err;
string errmsg = "Expected to get redis type " + std::to_string(expectedType) + " got type " + std::to_string(m_reply->type) + ", err: " + err;
Copy link
Contributor

Choose a reason for hiding this comment

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

std::

I am not sure if it will introduce ambiguity. Seems like you can remove std::.
Not a big issue.

@liuh-80
Copy link
Contributor Author

liuh-80 commented May 25, 2022

After integrate this change to sonic-buildimage repo, all E2E test passed: https://github.com/Azure/sonic-buildimage/pull/10825/checks

@liuh-80 liuh-80 marked this pull request as ready for review May 26, 2022 01:33
@liuh-80 liuh-80 merged commit 33afb9b into sonic-net:master May 26, 2022
liuh-80 added a commit to sonic-net/sonic-buildimage that referenced this pull request Jun 1, 2022
@liuh-80
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (#10825
96954f0 
)

#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: #10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.
    

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Build and install c++ version sonic-db-cli from swss-common.

#### 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/wiki/Configuration.
-->

#### A picture of a cute animal (not mandatory but encouraged)
liuh-80 added a commit to sonic-net/sonic-py-swsssdk that referenced this pull request Jun 1, 2022
@liuh-80
Remove sonic-db-cli (#122)
ca785a2 
#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Remove python version sonic-db-cli from swsssdk.

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Remove python version sonic-db-cli from swsssdk

#### 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/wiki/Configuration.
-->

#### A picture of a cute animal (not mandatory but encouraged)
liuh-80 added a commit to liuh-80/sonic-swss-common that referenced this pull request Jun 22, 2022
@liuh-80
Re-write sonic-db-cli with c++ for sonic startup performance issue (s…
1078bad 
…onic-net#607)

    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

    Re-write sonic-cli with c++.

    Add c++ unit test to cover all code.
    Pass all E2E test scenario.

<!--
- 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

    Re-write sonic-cli with c++ for sonic startup performance issue
liuh-80 added a commit to liuh-80/sonic-py-swsssdk that referenced this pull request Jun 22, 2022
@liuh-80
Remove sonic-db-cli (sonic-net#122)
4a004ed 
#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Remove python version sonic-db-cli from swsssdk.

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Remove python version sonic-db-cli from swsssdk

#### 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/wiki/Configuration.
-->

#### A picture of a cute animal (not mandatory but encouraged)
@liuh-80 liuh-80 mentioned this pull request Jun 22, 2022
Merged
liuh-80 added a commit that referenced this pull request Jun 23, 2022
@liuh-80
Re-write sonic-db-cli with c++ for sonic startup performance issue (#607
ace3a42 
) (#636)

Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

    Re-write sonic-cli with c++.

    Add c++ unit test to cover all code.
    Pass all E2E test scenario.

<!--
- 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

    Re-write sonic-cli with c++ for sonic startup performance issue
liuh-80 added a commit to sonic-net/sonic-py-swsssdk that referenced this pull request Jun 23, 2022
@liuh-80
Remove sonic-db-cli (#122) (#124)
4350b05 
#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Remove python version sonic-db-cli from swsssdk.

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Remove python version sonic-db-cli from swsssdk

#### 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/wiki/Configuration.
-->

#### A picture of a cute animal (not mandatory but encouraged)
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Jun 27, 2022
@liuh-80
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (son…
c580a13 
…ic-net#10825)

    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net#10218
    ETA of this issue will be 2022/05/31

    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.

    Pass all E2E test scenario.

<!--
- 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

    Build and install c++ version sonic-db-cli from swss-common.

<!--
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/wiki/Configuration.
-->
liuh-80 added a commit to sonic-net/sonic-buildimage that referenced this pull request Jun 30, 2022
@liuh-80
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (#10825
9b4387a 
) (#11262)

Fix sonic-db-cli high CPU usage on SONiC startup issue: #10218
    ETA of this issue will be 2022/05/31

    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.

    Pass all E2E test scenario.

<!--
- 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

    Build and install c++ version sonic-db-cli from swss-common.

<!--
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/wiki/Configuration.
-->
itamar-talmon pushed a commit to itamar-talmon/sonic-swss-common that referenced this pull request Jul 19, 2022
@liuh-80 @itamar-talmon
Re-write sonic-db-cli with c++ for sonic startup performance issue (s…
7baac15 
…onic-net#607)

#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++.

#### How to verify it
    Add c++ unit test to cover all code.
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Re-write sonic-cli with c++ for sonic startup performance issue
@liuh-80 liuh-80 mentioned this pull request Aug 1, 2022
Closed
yxieca pushed a commit that referenced this pull request Aug 8, 2022
@liuh-80 @yxieca
Re-write sonic-db-cli with c++ for sonic startup performance issue (#607
675d793 
)

#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net/sonic-buildimage#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++.

#### How to verify it
    Add c++ unit test to cover all code.
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Re-write sonic-cli with c++ for sonic startup performance issue
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Aug 12, 2022
@liuh-80
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (son…
88cd068 
…ic-net#10825)

    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net#10218
    ETA of this issue will be 2022/05/31

    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.

    Pass all E2E test scenario.

<!--
- 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

    Build and install c++ version sonic-db-cli from swss-common.

<!--
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/wiki/Configuration.
-->
liuh-80 added a commit to sonic-net/sonic-buildimage that referenced this pull request Aug 17, 2022
@liuh-80
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (#10825
6a2c540 
) (#11713)

Cherry pick PR #10825 to 202205 branch

#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: #10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.
    

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Build and install c++ version sonic-db-cli from swss-common.

#### 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/wiki/Configuration.
-->

#### 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
[swsscommon] Add c++ version sonic-db-cli from sonic-swss-common (son…
8d605e8 
…ic-net#10825) (sonic-net#11713)

Cherry pick PR sonic-net#10825 to 202205 branch

#### Why I did it
    Fix sonic-db-cli high CPU usage on SONiC startup issue: sonic-net#10218
    ETA of this issue will be 2022/05/31

#### How I did it
    Re-write sonic-cli with c++ in sonic-swss-common: sonic-net/sonic-swss-common#607
    Modify swss-common rules and slave.mk to install c++ version sonic-db-cli.
    

#### How to verify it
    Pass all E2E test scenario.

#### 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

#### Description for the changelog
    Build and install c++ version sonic-db-cli from swss-common.

#### 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/wiki/Configuration.
-->

#### A picture of a cute animal (not mandatory but encouraged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dprital dprital dprital left review comments

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

Assignees
No one assigned
Labels
Included in 202111 Branch Included in 202205 Branch Request for 202111 Branch Request for 202205 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liuh-80 @dprital @qiluo-msft @yxieca