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

[multi-asic] Fixed 13137 ERR python3: :- initializeGlobalConfig: SonicDBConfig Global config is already initialized #18609

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

wumiaont
Copy link
Contributor

@wumiaont wumiaont commented Apr 9, 2024

Issue:
#13137
If run "show interfaces status" we often see error in syslog "SonicDBConfig Global config is already initialized".

Analysis:
During investigation it is found the swsscommon.load_sonic_global_db_config() has been triggered multiple times. Especially in a multi Asic scenario.

CLI triggers "intfutil -c status" with a separate process. Within the process it will trigger IntfStatus init --> MultiAsic() init-->load_db_config()-->load_sonic_global_db_config(). This time initializeGlobalConfig() will run to the end as m_global_init flag is false.

CLI will also run get_port_config() --> db_connect_configdb() --> load_sonic_global_db_config() (for different namepaces. In our case is asic0 and asic1). This time m_global_init flag is true which causes error log and return.

Solution:
There was a PR in master already fixed this issue(#10960). Cherry-picked this back to 202205. Added checks in 2 other places where load_sonic_global_db_config is called without check in the code and was not covered by PR 10960.

Testing
Run on chassis with 202205 loaded. Loaded the python files modified with this PR into the chassis. Run "show interface status" and will not see the error log. Show command has interface status results correctly.

…10960)

Why I did it
Provide fix for comment: https://github.com/Azure/sonic-buildimage/pull/10475/files#r847753187;
Move laoding database config to application code instead of portconfig as portconfig is used as a library.

How I did it
Remove try exception handing from portconfig.py during config_db intialization.
Move loading of database config to application that uses portconfig.py.

How to verify it
unit-test passes.
Verified that it does not cause issue during boot up of multi-asic VS image.
Verified that config_db generation was successful in multi-asic VS.
@wumiaont
Copy link
Contributor Author

wumiaont commented Apr 9, 2024

This is a cherry-pick fix from master. Added 2 more places with check where load_sonic_global_db_config() is called without check.

@wumiaont wumiaont changed the title [portconfig]: Remove try exception during config_db initialization. (… [multi-asic] Fixed 13137 ERR python3: :- initializeGlobalConfig: SonicDBConfig Global config is already initialized Apr 9, 2024
@wumiaont
Copy link
Contributor Author

@qiluo-msft Please review. Thx

@qiluo-msft
Copy link
Collaborator

The code lgtm. However you should fix the master completely and backport whatever commits to 202205. Not to submit PR to 202205 directly.

@wumiaont
Copy link
Contributor Author

The code lgtm. However you should fix the master completely and backport whatever commits to 202205. Not to submit PR to 202205 directly.

OK. I will create PR against master soon.

@wumiaont
Copy link
Contributor Author

@qiluo-msft @rlhui Created PR against master #18691. Please review. Unfortunately, that PR cannot be cherry-pick to 202205 as it's missing another important fix with original PR 10960 (change in portconfig.py). So best way is to let this in 202205 and have 18691 to handle fix for master.

@deepak-singhal0408
Copy link
Contributor

MSFT ADO: 25516271

@deepak-singhal0408
Copy link
Contributor

@rlhui , could you pl help merge this change? Thanks!

@rlhui rlhui merged commit 87a5820 into sonic-net:202205 Apr 17, 2024
17 checks passed
@gechiang
Copy link
Collaborator

@deepak-singhal0408 why is the label requested for 202305 is applied here but not from the master PR (#10960) instead? This is not usually how it works...
Is the additional fix also needed for master? If so, please raise another PR in master so that the master has all the fixes and request the back port from 10960 and the new PR that you will be raising. I will remove the label "request for 202305" from this one for now.

@deepak-singhal0408
Copy link
Contributor

@deepak-singhal0408 why is the label requested for 202305 is applied here but not from the master PR (#10960) instead? This is not usually how it works... Is the additional fix also needed for master? If so, please raise another PR in master so that the master has all the fixes and request the back port from 10960 and the new PR that you will be raising. I will remove the label "request for 202305" from this one for now.

sounds good.. actually earlier there was only one PR in 202205, and we were under the impression that nothing is needed in master.. thats why I added the label here.. but later on, we made a change for this in master as well.. so yes, ideally the label should be put on master branch PR.. thanks for fixing this.

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

Successfully merging this pull request may close these issues.

7 participants