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

[flex-counters] Delay flex counters stats init for faster boot time #1646

Merged
merged 7 commits into from
Apr 28, 2021
Merged

[flex-counters] Delay flex counters stats init for faster boot time #1646

merged 7 commits into from
Apr 28, 2021

Conversation

shlomibitton
Copy link
Contributor

@shlomibitton shlomibitton commented Feb 18, 2021

Signed-off-by: Shlomi Bitton shlomibi@nvidia.com

What I did
Update flex counters DB with counters stats only when counters are enabled.
As long as the polling counters are not enabled, flex counters information will stored internally on PortsOrch.

Why I did it
Creating flex counters objects on the DB will trigger 'SYNCD' to access the HW for query statistics capabilities.
This HW access takes time and will be better to finish boot before doing this (mainly for fast-reboot but good to have in general).
The flex counters are not crucial at boot time, we can delay it to the end of the boot process.

How I verified it
Reboot a switch and observer the flex counters DB populated after counters are enabled.

Details if related

orchagent/portsorch.cpp Show resolved Hide resolved
orchagent/portsorch.cpp Show resolved Hide resolved
orchagent/portsorch.cpp Show resolved Hide resolved
tests/mock_tests/portsorch_ut.cpp Show resolved Hide resolved
@liat-grozovik
Copy link
Collaborator

@stepanblyschak could u please help to review?

stepanblyschak
stepanblyschak previously approved these changes Mar 2, 2021
@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

liat-grozovik
liat-grozovik previously approved these changes Mar 7, 2021
@liat-grozovik
Copy link
Collaborator

@shlomibitton please check test failure on PG counters. this is the only test i think relevant to your changes.

@liat-grozovik
Copy link
Collaborator

/AzurePipleines run

Shlomi Bitton added 3 commits March 29, 2021 18:24
Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
Fix 'test_pg_drop' to adapt new change

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
@liat-grozovik
Copy link
Collaborator

@stepanblyschak any comments?

@liat-grozovik
Copy link
Collaborator

@stepanblyschak kindly please review. your approval is needed for such change.

@@ -0,0 +1,83 @@
import time
Copy link
Collaborator

Choose a reason for hiding this comment

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

@shlomibitton could you please provide description of the flows to be tested in this unit test? it will help to ensure all cases are handled as part of the ut

Copy link
Contributor Author

@shlomibitton shlomibitton Apr 4, 2021

Choose a reason for hiding this comment

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

@liat-grozovik Added the test flow and description to the PR, please review.

liat-grozovik
liat-grozovik previously approved these changes Apr 4, 2021
tests/test_flex_counters.py Outdated Show resolved Hide resolved
tests/test_flex_counters.py Outdated Show resolved Hide resolved
tests/test_flex_counters.py Outdated Show resolved Hide resolved
@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1646 in repo Azure/sonic-swss

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik liat-grozovik requested review from qiluo-msft, yxieca and judyjoseph and removed request for qiluo-msft April 7, 2021 05:09
@liat-grozovik
Copy link
Collaborator

@judyjoseph and @yxieca would you like to review this PR as well?
we would recommend to have it as part of 202012 to reduce time on boot which can definitely help for fastboot as well.

@shlomibitton shlomibitton requested a review from yxieca April 11, 2021 06:22
@liat-grozovik
Copy link
Collaborator

@yxieca and @judyjoseph can you please review?

@daall
Copy link
Contributor

daall commented Apr 29, 2021

@shlomibitton there are merge conflicts w/ the 202012 branch, please submit a PR against 202012 for this change.

lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Apr 29, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1646)
[routeorch] Add support for blackhole routes (sonic-net/sonic-swss#1723)
Update pool sizes during initialization from timer only (sonic-net/sonic-swss#1708)

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
@shlomibitton
Copy link
Contributor Author

shlomibitton commented May 2, 2021

@shlomibitton there are merge conflicts w/ the 202012 branch, please submit a PR against 202012 for this change.

@daall I have created a new PR against 202012 #1736

qiluo-msft added a commit that referenced this pull request May 12, 2021
liat-grozovik pushed a commit that referenced this pull request May 13, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1646)
[routeorch] Add support for blackhole routes (sonic-net/sonic-swss#1723)
Update pool sizes during initialization from timer only (sonic-net/sonic-swss#1708)

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
@shlomibitton shlomibitton deleted the shlomi_delay_counters_2_master branch July 27, 2021 16:32
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
[flex-counters] Delay flex counters stats init for faster boot time (sonic-net/sonic-swss#1646)
[routeorch] Add support for blackhole routes (sonic-net/sonic-swss#1723)
Update pool sizes during initialization from timer only (sonic-net/sonic-swss#1708)

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…onic-net#1646)

What I did
Update flex counters DB with counters stats only when counters are enabled.
As long as the polling counters are not enabled, flex counters information will stored internally on PortsOrch.

Why I did it
Creating flex counters objects on the DB will trigger 'SYNCD' to access the HW for query statistics capabilities.
This HW access takes time and will be better to finish boot before doing this (mainly for fast-reboot but good to have in general).
The flex counters are not crucial at boot time, we can delay it to the end of the boot process.

How I verified it
Reboot a switch and observer the flex counters DB populated after counters are enabled.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
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.

5 participants