[db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches #993

stephenxs · 2020-07-17T05:44:44Z

This is the updated version of #990. As the original branch has been forcely updated, we need to open a new PR for it.
All comments in that PR have been addressed in this one.

- What I did

Move the code for Mellanox buffer configuration migrating to a separate file.
The motivation is not to put too much code in db_migrator.py which can distract others from the main logic.
Reorganize the code in a graceful and easy-to-maintain way.
- Extract all the buffer configuration into a separate function so that it can be used in different rounds of migrations.
  The buffer configuration of each database version will be used twice:
  - the time when migrating from the old version to the current version
  - the time when from the current version to the new version.
- The logic of Mellanox buffer migrator is:
  1. Compare the current buffer configuration with the default settings
  2. If they match, map the old value to the new one
  3. Insert the new setting into CONFIG_DB.
- The granularity of matching is:
  - For buffer pool, the configuration of all pools is compared as a whole, which means the update is performed only if the configuration of all pools matches the default value.
  - For buffer profile for per port reserved ingress/egress buffer, the configuration of all profiles are compared as a whole, which means the update is performed only if the configuration of all that kind of profiles matches the default value
  - For profile for lossless PGs, each profile of a speed, cable length pair is compared separately.
Support migrating database regarding buffer configuration (database version 1.0.4) for all Mellanox switches

Signed-off-by: Stephen Sun stephens@mellanox.com

- How I did it

- How to verify it

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

db_migrator supports migrating old configuration who has 2 ingress pools into the new configuration who has 1 ingress pool, including BUFFER_POOL, BUFFER_PROFILE and BUFFER_PORT_INGRESS_PROFILE_LIST Signed-off-by: Stephen Sun <stephens@mellanox.com>

1. Don't need to migrate for single buffer pool mode 2. Move buffer setting migration to another file 3. Remove unnecessary code/upgrading flows Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-17T05:51:45Z

This pull request introduces 2 alerts and fixes 1 when merging e16a8e7 into e3d6ba0 - view on LGTM.com

new alerts:

1 for Unused local variable
1 for Unused argument in a formatting call

fixed alerts:

1 for Unused local variable

Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-17T06:12:39Z

This pull request fixes 1 alert when merging 955eac2 into e3d6ba0 - view on LGTM.com

fixed alerts:

1 for Unused local variable

Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-17T07:28:36Z

This pull request fixes 1 alert when merging daf8b8a into e3d6ba0 - view on LGTM.com

fixed alerts:

1 for Unused local variable

scripts/mellanox_db_migrator.py

lgtm-com · 2020-07-18T01:22:52Z

This pull request fixes 1 alert when merging 36ee64e into e3d6ba0 - view on LGTM.com

fixed alerts:

1 for Unused local variable

This issue can fail warm reboot because after warm reboot the buffermgr doesn't have time to generate lossless profiles and the following orchagent bake can fail due to this Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-18T14:58:37Z

This pull request fixes 1 alert when merging 30f7faa into e3d6ba0 - view on LGTM.com

fixed alerts:

1 for Unused local variable

keboliu · 2020-07-20T05:59:47Z

retest this please

liat-grozovik

please extend your PR message with more information about motivation, which SKUs are affected and how to have this new values enforced.

prsunny · 2020-07-21T00:37:31Z

retest this please

liat-grozovik · 2020-07-21T06:39:14Z

retest this please

stephenxs · 2020-07-21T08:42:02Z

please extend your PR message with more information about motivation, which SKUs are affected and how to have this new values enforced.

Done.

liat-grozovik · 2020-07-21T19:37:35Z

Retest this please

prsunny

lgtm, @neethajohn to review the buffer pool values

jleveque · 2020-07-22T01:31:13Z

Retest this please

Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-22T08:51:46Z

This pull request fixes 1 alert when merging e28c823 into 995cf39 - view on LGTM.com

fixed alerts:

1 for Unused local variable

Signed-off-by: Stephen Sun <stephens@mellanox.com>

lgtm-com · 2020-07-29T07:25:07Z

This pull request fixes 1 alert when merging 2a7b809 into aa1b072 - view on LGTM.com

fixed alerts:

1 for Unused local variable

lgtm-com · 2020-08-04T23:14:27Z

This pull request fixes 1 alert when merging e256e93 into 5d26391 - view on LGTM.com

fixed alerts:

1 for Unused local variable

stephenxs · 2020-08-04T23:42:30Z

Hi @prsunny , @neethajohn ,
All comments are addressed. Could you please approve and merge?
Thank you!
Stephen

liat-grozovik · 2020-08-06T08:47:33Z

@neethajohn could you please review and see if this can be merged?
we would like to have it also on 201911 and to make sure that if someone is upgrading from earlier image w/o the new buffer calculation will get the new values without doing any manual work, so it will be just simply working for users who are not going via the minigraph reload approach each upgrade.

lgtm-com · 2020-08-10T06:34:26Z

This pull request introduces 1 alert and fixes 1 when merging 488173d into 0225c09 - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Unused local variable

liat-grozovik · 2020-08-10T17:54:29Z

retest this please

Signed-off-by: Stephen Sun <stephens@mellanox.com>

stephenxs · 2020-08-11T11:08:13Z

Hi @prsunny @neethajohn,
I push another commit, adjusting the code according to the latest db_migrator change.
Please review. Thanks.

lgtm-com · 2020-08-11T11:10:44Z

This pull request fixes 1 alert when merging 66229e5 into 7ae8024 - view on LGTM.com

fixed alerts:

1 for Unused local variable

stephenxs · 2020-08-11T23:21:23Z

retest this please

…on for all Mellanox switches This is to backport PR sonic-net#993 to 201911 Signed-off-by: Stephen Sun <stephens@nvidia.com>

…on for all Mellanox switches (#1053) This is to backport PR #993 to 201911 Signed-off-by: Stephen Sun <stephens@nvidia.com>

abdosi · 2020-09-03T23:02:51Z

@stephenxs Please create PR for 201911

Stephen Sun added 2 commits July 17, 2020 02:31

Update according to MSFT comments

e16a8e7

1. Don't need to migrate for single buffer pool mode 2. Move buffer setting migration to another file 3. Remove unnecessary code/upgrading flows Signed-off-by: Stephen Sun <stephens@mellanox.com>

stephenxs mentioned this pull request Jul 17, 2020

[Mellanox] db migrator for buffer configuration stephenxs/sonic-utilities#8

Closed

Fix LGTM warning

955eac2

Signed-off-by: Stephen Sun <stephens@mellanox.com>

Fix an error

daf8b8a

Signed-off-by: Stephen Sun <stephens@mellanox.com>

prsunny reviewed Jul 17, 2020

View reviewed changes

scripts/mellanox_db_migrator.py Outdated Show resolved Hide resolved

mellanox_db_migrator => mellanox_buffer_migrator

36ee64e

Fix issue: after migration the lossless profiles are lost

30f7faa

This issue can fail warm reboot because after warm reboot the buffermgr doesn't have time to generate lossless profiles and the following orchagent bake can fail due to this Signed-off-by: Stephen Sun <stephens@mellanox.com>

liat-grozovik reviewed Jul 20, 2020

View reviewed changes

stephenxs mentioned this pull request Jul 21, 2020

[Mellanox] Update the buffer setting sonic-net/sonic-buildimage#4989

Merged

prsunny previously approved these changes Jul 21, 2020

View reviewed changes

stephenxs marked this pull request as draft July 22, 2020 06:30

Update buffer configuration for version 1.0.4

e28c823

Signed-off-by: Stephen Sun <stephens@mellanox.com>

stephenxs dismissed prsunny’s stale review via e28c823 July 22, 2020 08:43

Update the buffer setting for version 1.0.4

2a7b809

Signed-off-by: Stephen Sun <stephens@mellanox.com>

stephenxs marked this pull request as ready for review July 29, 2020 07:35

liat-grozovik requested a review from neethajohn July 30, 2020 11:29

stephenxs requested review from prsunny and neethajohn August 4, 2020 23:09

prsunny previously approved these changes Aug 5, 2020

View reviewed changes

liat-grozovik added the Request for 201911 Branch label Aug 6, 2020

neethajohn previously approved these changes Aug 7, 2020

View reviewed changes

Merge branch 'master' into db_migrator-general

488173d

[db_migrator] Adjust db_migrator according to the latest master change

66229e5

Signed-off-by: Stephen Sun <stephens@mellanox.com>

stephenxs dismissed stale reviews from neethajohn and prsunny via 66229e5 August 11, 2020 11:03

stephenxs requested review from prsunny and neethajohn August 11, 2020 11:08

neethajohn approved these changes Aug 13, 2020

View reviewed changes

liat-grozovik merged commit d5fdd74 into sonic-net:master Aug 13, 2020

stephenxs mentioned this pull request Aug 14, 2020

[sonic-utilities] Advance submodule sonic-net/sonic-buildimage#5184

Merged

3 tasks

stephenxs deleted the db_migrator-general branch August 14, 2020 03:18

stephenxs mentioned this pull request Aug 14, 2020

[db_migrator] Backport db_migrator PR 993 to 201911 #1053

Merged

abdosi pushed a commit that referenced this pull request Aug 21, 2020

[db_migrator] Support migrating database regarding buffer configurati…

323073e

…on for all Mellanox switches (#1053) This is to backport PR #993 to 201911 Signed-off-by: Stephen Sun <stephens@nvidia.com>

abdosi removed the Request for 201911 Branch label Sep 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches #993

[db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches #993

stephenxs commented Jul 17, 2020 •

edited

Loading

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 18, 2020

lgtm-com bot commented Jul 18, 2020

keboliu commented Jul 20, 2020

liat-grozovik left a comment

prsunny commented Jul 21, 2020

liat-grozovik commented Jul 21, 2020

stephenxs commented Jul 21, 2020

liat-grozovik commented Jul 21, 2020

prsunny left a comment

jleveque commented Jul 22, 2020

lgtm-com bot commented Jul 22, 2020

lgtm-com bot commented Jul 29, 2020

lgtm-com bot commented Aug 4, 2020

stephenxs commented Aug 4, 2020

liat-grozovik commented Aug 6, 2020

lgtm-com bot commented Aug 10, 2020

liat-grozovik commented Aug 10, 2020

stephenxs commented Aug 11, 2020 •

edited

Loading

lgtm-com bot commented Aug 11, 2020

stephenxs commented Aug 11, 2020

abdosi commented Sep 3, 2020

[db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches #993

[db_migrator] Support migrating database regarding buffer configuration for all Mellanox switches #993

Conversation

stephenxs commented Jul 17, 2020 • edited Loading

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 17, 2020

lgtm-com bot commented Jul 18, 2020

lgtm-com bot commented Jul 18, 2020

keboliu commented Jul 20, 2020

liat-grozovik left a comment

Choose a reason for hiding this comment

prsunny commented Jul 21, 2020

liat-grozovik commented Jul 21, 2020

stephenxs commented Jul 21, 2020

liat-grozovik commented Jul 21, 2020

prsunny left a comment

Choose a reason for hiding this comment

jleveque commented Jul 22, 2020

lgtm-com bot commented Jul 22, 2020

lgtm-com bot commented Jul 29, 2020

lgtm-com bot commented Aug 4, 2020

stephenxs commented Aug 4, 2020

liat-grozovik commented Aug 6, 2020

lgtm-com bot commented Aug 10, 2020

liat-grozovik commented Aug 10, 2020

stephenxs commented Aug 11, 2020 • edited Loading

lgtm-com bot commented Aug 11, 2020

stephenxs commented Aug 11, 2020

abdosi commented Sep 3, 2020

stephenxs commented Jul 17, 2020 •

edited

Loading

stephenxs commented Aug 11, 2020 •

edited

Loading