Skip to content

[code sync] Merge code from sonic-net/sonic-utilities:202511 to 202512 - #452

Merged
mssonicbld merged 2 commits into
Azure:202512from
mssonicbld:sonicbld/202512-merge
Sep 10, 2026
Merged

[code sync] Merge code from sonic-net/sonic-utilities:202511 to 202512#452
mssonicbld merged 2 commits into
Azure:202512from
mssonicbld:sonicbld/202512-merge

Conversation

@mssonicbld

Copy link
Copy Markdown
Collaborator
* ea7ce87a - (origin/202511) [GCU] Performance rework of the patch sorter (#3831) [202511] (#4810) (2026-09-09) [rimunagala]<br>```

rimunagala and others added 2 commits September 9, 2026 08:59
* Merge pull request #3831 from bhouse-nexthop/bhouse-nexthop/gcu-perf

Generic Configuration Updater (GCU) performance enhancements

Generic Configuration Updater is extremely slow, using the python profiler it was possible to determine the worst offenders where changes could be made without affecting the overall algorithm and HLD design documentation.

Brief overview of changes:
* Prevent copy.deepcopy() calls where possible
* Don't run validation twice back to back
* Move configdb path <> xpath conversion logic to sonic-yang-mgmt where it belongs and enhance it to support schema conversion (not just data) and add caching.
* Sort table keys by the number of schema backlinks and must statements for the node to try better guess the right order of the patches to generate rather than doing it in alphabetical order which is likely to cause validation failures.
* Add ability to Group patches together in some commits where its known they will not cause issues, these are things like grouping parameter updates under the same key.
* When applying changes, do not re-read the configuration from redis twice between each applied patch (this is **extremely** slow, and actually hid a race condition).  We are mutating the configuration and a lock is held so we know the expected before and after.  There is still a final validation to ensure something didn't go sideways.

Dependencies:
 * sonic-yang-mgmt enhancements: sonic-net/sonic-buildimage#22254
 * sonic-yang-mgmt parse uses/grouping: sonic-net/sonic-buildimage#21907
 * sonic-utilities rely on sonic-yang-mgmt uses/grouping handling: sonic-net/sonic-utilities#3814

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).

(cherry picked from commit bd3de9d)
Signed-off-by: rimunagala <rimunagala@microsoft.com>

* [GCU] Make find_ref_paths load YANG data when nothing is loaded (202511-only)

202511-only hardening. Not required on master.

PR #3831 adds a `reload_config` gate to PathAddressing.find_ref_paths() so that
bulk operations skip redundant sy.loadData() calls:

    if reload_config:
        sy.loadData(config)

That gate was authored against master's tree, where PR #4118 ("Remove direct
dependency on libyang", 2026-04-27) had already deleted _get_inner_leaf_xpaths()
and with it the only sy.root dereference in this function. On master, skipping
the load is therefore harmless by construction.

202511 does not contain #4118 and retains _get_inner_leaf_xpaths(), which does:

    nodes = sy.root.find_path(xpath).data()

so on this branch find_ref_paths() depends on YANG data already being loaded.
That requirement is satisfied today only by statement ordering: every caller
(patch_sorter.py:757, 991, 1687) seeds reload_config=True and flips it to False
only after the first call has loaded. sy is a process-lifetime singleton
(create_sonic_yang_with_loaded_models() calls loadYangModel() once and never
loadData()), so sy.root is None only before the first-ever load in the process.

The invariant holds today, but it is implicit, undocumented, and not something
master has any reason to preserve. Any future reordering, new caller, or new
move generator that reaches a reload_config=False call site first would fail
with AttributeError: 'NoneType' object has no attribute 'find_path'.

Make the requirement explicit instead of relying on call order:

    if reload_config or sy.root is None:

The #4476 config-hash caching is preserved, so redundant loads are still
skipped.

No behavioural change is expected or observed. On cisco-8000 (56 ports, 6 ACL
tables) the pre-fix and post-fix patched arms are equal within noise --
0.74/0.75/14.31/2.00s vs 0.73/0.76/14.09/2.11s -- with identical move counts.
460 unit tests + 81 subtests pass.

NOTE: this change is defensive. No production failure has been reproduced. An
instrumented end-to-end sort() of a create-only PORT lanes change on a port
referenced by an ACL ports leaf-list reaches the reload_config=False sites only
after a load has already happened, and behaves identically with and without
this change.

The alternative -- backporting #4118 -- was evaluated and rejected: it touches
config/config_mgmt.py and sonic_package_manager/manager.py and adds a semgrep
CI gate, all outside the GCU-only scope agreed for this backport.

Signed-off-by: rimunagala <rimunagala@microsoft.com>

---------

Signed-off-by: rimunagala <rimunagala@microsoft.com>
Co-authored-by: Guohan Lu <lguohan@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator Author

/azp run

@mssonicbld
mssonicbld merged commit 7dd444a into Azure:202512 Sep 10, 2026
3 checks passed
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.

2 participants