Skip to content

[GCU] Performance improvement by making patch sorting optional#2764

Merged
isabelmsft merged 4 commits intosonic-net:masterfrom
isabelmsft:yang_gcu_perf_improvement
Mar 31, 2023
Merged

[GCU] Performance improvement by making patch sorting optional#2764
isabelmsft merged 4 commits intosonic-net:masterfrom
isabelmsft:yang_gcu_perf_improvement

Conversation

@isabelmsft
Copy link
Copy Markdown
Contributor

@isabelmsft isabelmsft commented Mar 29, 2023

What I did

Make patch sorting optional.

Why: In some cases, patch sorting is unnecessary, as the patch doesn't have dependencies on other tables, and the patch is already in an order that can directly be applied to ConfigDB. A specific example is when GCU is used for YANG validation of direct writes to ConfigDB via set_entry or mod_entry. In these scenarios, we can directly apply each element in the JsonPatch without need for additional sorting

This allows for a performance improvement of ~40%.

Sample command to add portchannel including unnecessary patch sorting:

admin@sonic:~$ sudo config portchannel add PortChannel09
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "add", "path": "/PORTCHANNEL/PortChannel09", "value": {}}, {"op": "add", "path": "/PORTCHANNEL/PortChannel09", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was converted into 1 change:
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel09", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Applying 1 change in order:
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel09", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
[{"op": "add", "path": "/PORTCHANNEL/PortChannel09", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
**Elapsed time: 4.543376445770264**

Sample command to add portchannel without unnecessary patch sorting:

admin@sonic:~$ sudo config portchannel add PortChannel06
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {}}, {"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Converting patch to JsonChange.
Patch Applier: The patch was converted into 2 changes:
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {}}]
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Applying 2 changes in order:
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {}}]
[{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {}}]
Patch Applier:   * [{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
[{"op": "add", "path": "/PORTCHANNEL/PortChannel06", "value": {"admin_status": "up", "mtu": "9100", "lacp_key": "auto", "min_links": "1"}}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
**Elapsed time: 2.747215747833252**

How I did it

In ValidatedConfigDBConnector decorator code, set the sort=False flag, and read this value in GCU to determine whether patch sorting is necessary

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)

@isabelmsft isabelmsft force-pushed the yang_gcu_perf_improvement branch 2 times, most recently from f7a68e0 to 5dbd7bb Compare March 29, 2023 05:21
@isabelmsft isabelmsft changed the title [GCU] Optional Performance improvement by making patch sorting optional [GCU] Performance improvement by making patch sorting optional Mar 29, 2023
@isabelmsft isabelmsft force-pushed the yang_gcu_perf_improvement branch from 5dbd7bb to 6583b07 Compare March 29, 2023 05:46
@isabelmsft isabelmsft requested review from qiluo-msft and wen587 March 29, 2023 20:45
wen587
wen587 previously approved these changes Mar 30, 2023
View reviewed changes
Comment thread config/validated_config_db_connector.py
qiluo-msft
qiluo-msft previously approved these changes Mar 30, 2023
View reviewed changes
@isabelmsft isabelmsft dismissed stale reviews from qiluo-msft and wen587 via eaf8b51 March 31, 2023 20:20
@isabelmsft isabelmsft merged commit e6f9f46 into sonic-net:master Mar 31, 2023
@ayurkiv-nvda ayurkiv-nvda mentioned this pull request Apr 18, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@wen587 wen587 wen587 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@isabelmsft @wen587 @qiluo-msft