[action] [PR:22254] sonic-yang-mgmt: Generic Config Updater - performance dependencies#1711
Merged
mssonicbld merged 1 commit intoAzure:202412from Oct 19, 2025
Merged
Conversation
#### Why I did it ***Important***: Please review each commit (including commit message) individually. Looking at the patch-set as a whole may cause confusion. Generic Config Updater (GCU) is notoriously slow. These patches add some helpers for the GCU overhaul (mostly in sonic-utilities) in order to facilitate the optimizations. These changes are in sonic-yang-mgmt plus a patch to libyang 1. Changes include: * Libyang v1 was not exposing `must` data for leaf nodes (like it does for other node types). Patch to correct this oversight. * Loading of sonic configuration data should not mutate the user-provided data, this forces callers to know they need to deepcopy the data, plus in most instances data won't be mutated. * sonic-yang-mgmt test models should more closely mimic real sonic yang models as we can't implement other features that assume this otherwise (sonic mandates that the top-level container has the same name as the module). * Import `sonic configdb`<>`yang xpath` conversion from sonic-utilities as this should be shared code. 90% of this code is copied from the original source but does contain some bugfixes and enhancements including caching. * Make `find_schema_dependencies()` public, plus add the ability to find dependencies recursively. This implementation is caching. * Add new `find_schema_must_count()`, with recursive capabilities to find if a node (and its children) have must clauses. This implementation is caching. sonic-utilities PR: sonic-net/sonic-utilities#3831 Stats below ... (stats need both this and the sonic-utilities PR to be relevant)... <ins>**Original Performance:**</ins> Dry Run: ``` time sudo config replace -d ./config_db.json ... real 2m51.588s user 2m23.777s sys 0m25.300s ``` Full: ``` time sudo config replace ./config_db.json ... real 14m53.772s user 12m2.376s sys 2m8.908s ``` <ins>**With Patch**:</ins> Dry Run: ``` time sudo config replace -d ./config_db.json ... real 0m59.602s user 0m56.434s sys 0m2.110s ``` Full: ``` time sudo config replace ./config_db.json ... real 1m54.303s user 0m58.482s sys 0m2.545s ``` So that's roughly 3x improvement for dry-run, and 7.5x improvement for full commit. There is room for improvement on the full commit due to a `sleep(1)` being used between each patch because of a race condition found in the prior code (that was hidden due to a costly sanity check that has been removed). ##### Work item tracking #### How I did it Gathered profiling data using cProfile and evaluated where the largest gains could be had. #### How to verify it This patch is standalone as it will not cause any issues in other projects which use sonic-yang-mgmt or libyang, however the performance benefits are in sonic-utilities. Apply both this commit and the sonic-utilities PR to a local branch, build and run sonic-utilities tests. Then create a full image, load it onto a DUT (with default configuration), and use the attached [config_db.json](https://github.com/user-attachments/files/19635712/config_db.json) to attempt a `config replace` operation (tested on Dell S5248F). #### Which release branch to backport (provide reason below if selected) - [x] 202411 #### Tested branch (Please provide the tested image version) master as of 20250521 #### Description for the changelog sonic-yang-mgmt: Generic Config Updater - performance dependencies #### Link to config_db schema for YANG module changes N/A #### A picture of a cute animal (not mandatory but encouraged) Fixes sonic-net/sonic-buildimage#22372 Signed-off-by: Brad House <bhouse@nexthop.ai>
Collaborator
Author
|
Original PR: sonic-net/sonic-buildimage#22254 |
Collaborator
Author
|
/azp run |
Merged
1 task
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
bgp sanity check failing during the test, likely testbed problem: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
Important: Please review each commit (including commit message) individually. Looking at the patch-set as a whole may cause confusion.
Generic Config Updater (GCU) is notoriously slow. These patches add some helpers for the GCU overhaul (mostly in sonic-utilities) in order to facilitate the optimizations. These changes are in sonic-yang-mgmt plus a patch to libyang 1.
Changes include:
mustdata for leaf nodes (like it does for other node types). Patch to correct this oversight.sonic configdb<>yang xpathconversion from sonic-utilities as this should be shared code. 90% of this code is copied from the original source but does contain some bugfixes and enhancements including caching.find_schema_dependencies()public, plus add the ability to find dependencies recursively. This implementation is caching.find_schema_must_count(), with recursive capabilities to find if a node (and its children) have must clauses. This implementation is caching.sonic-utilities PR: sonic-net/sonic-utilities#3831
Stats below ... (stats need both this and the sonic-utilities PR to be relevant)...
Original Performance:
Dry Run:
Full:
With Patch:
Dry Run:
Full:
So that's roughly 3x improvement for dry-run, and 7.5x improvement for full commit. There is room for improvement on the full commit due to a
sleep(1)being used between each patch because of a race condition found in the prior code (that was hidden due to a costly sanity check that has been removed).Work item tracking
How I did it
Gathered profiling data using cProfile and evaluated where the largest gains could be had.
How to verify it
This patch is standalone as it will not cause any issues in other projects which use sonic-yang-mgmt or libyang, however the performance benefits are in sonic-utilities. Apply both this commit and the sonic-utilities PR to a local branch, build and run sonic-utilities tests. Then create a full image, load it onto a DUT (with default configuration), and use the attached
config_db.json to attempt a
config replaceoperation (tested on Dell S5248F).Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
master as of 20250521
Description for the changelog
sonic-yang-mgmt: Generic Config Updater - performance dependencies
Link to config_db schema for YANG module changes
N/A
A picture of a cute animal (not mandatory but encouraged)
Fixes sonic-net/sonic-buildimage#22372
Signed-off-by: Brad House bhouse@nexthop.ai