Skip to content

Ygot diff tool#103

Merged
anand-kumar-subramanian merged 5 commits intosonic-net:masterfrom
sachinholla:ygotdiff
May 22, 2023
Merged

Ygot diff tool#103
anand-kumar-subramanian merged 5 commits intosonic-net:masterfrom
sachinholla:ygotdiff

Conversation

@sachinholla
Copy link
Copy Markdown
Contributor

Why I did it

Subscription enhancements proposed in sonic-net/SONiC#1287 requires gNMI server to create a SAMPLE notification message by comparing YGOT objects from two successive iterations. We observed that the standard ygot.Diff API takes more time and memory when the YGOT objects have multiple child objects. Hence developed an in-house diff API

How I did it

Performance issues of ygot.Diff API are mainly due to it flattening the YGOT objects into list of {leaf_path, leaf_value} pairs and then comparing those lists. The in-house Diff API avoids this flattening step. It traverses both YGOT object attributes together, compares the values inline and records the changes if any. Following table captures the time taken (in nanoseconds) by ygot.Diff and the in-house diff APIs for diffing two openconfig-acl objects. Size "MxN" stands for ygot object with M ACLs and N rules in each ACL. All values are in nanoseconds.

Test case API size=10x10 size=10x100 size=100x1000
No changes ygot.Diff 9024089697 854023452896 OOM
in-house Diff 1784778 15460520 1757874254
One attr changed ygot.Diff 9285911808 857292949475 OOM
in-house Diff 1547512 16506496 1786245108
All attrs changed ygot.Diff 9333680743 855581319202 OOM
in-house Diff 3562498 34306626 3846524858
Create all attrs ygot.Diff 80830018 858752644
in-house Diff 7101530 74663888 8245685970
Delete all attrs ygot.Diff 83082154 755762682
in-house Diff 120232 125293 453306

How to verify it

Tested the functionality through gotests that handcraft two ygot objects, invoke the new Diff function and verify the results.
The gNMI server code to use this API is not ready as of now.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Standard ygot.Diff API flattens both structs into {path, value} pair
list and compares them to resolve new, modified and deleted values.
This approach does not scale well for large ygot trees. Following are
the go benchmarks for ygot.Diff API with openconfig-acl ygot trees.
Size "MxN" stands for ygot tree with M ACLs and N rules in each ACL.

+-------------------+---------------+----------------+
|   Test case       |  size=10x10   |  size=10x100   |
+-------------------+---------------+----------------+
| No changes        | 9024089697ns  | 854023452896ns |
| One attr changed  | 9285911808ns  | 857292949475ns |
| All attrs changed | 9333680743ns  | 855581319202ns |
| Create all attrs  |   80830018ns  |    858752644ns |
| Delete all attrs  |   83082154ns  |    755762682ns |
+-------------------+---------------+----------------+

Implemented a custom ygot diff tool to compare the ygot struct fields
inline. Uses reflection to traverse and compare structs.
Function signature:

func Diff(s1, s2 ygot.GoStruct, opt DiffOptions) (DiffResults, error)

type DiffOptions struct {
    RecordAll bool // include all s2 attrs, even if not modified
}

type DiffResults struct {
    Update []*gnmi.Update
    Delete []*gnmi.Path
}

Diff function will be used for generating notification messages for
SMPLE subscriptions -- by comparing ygot objects from the previous
iteration with the current objects. By default, Diff returns delete
paths and update values for new/modified fields only. Suitable for
SAMPLE subscription processing when suppress_redundant=true. The
suppress_redundant=false case requires all set attributes of the ygot
struct as update values (even if they are not modified). This can be
achieved by passing RecordAll=true in the DiffOptions.

Go benchmark for the new Diff API with openconfig-acl model:
+-------------------+------------+-------------+---------------+
|   Test case       | size=10x10 | size=10x100 | size=100x1000 |
+-------------------+---------------+--------------------------+
| No changes        | 1784778ns  | 15460520ns  | 1757874254ns  |
| One attr changed  | 1547512ns  | 16506496ns  | 1786245108ns  |
| All attrs changed | 3562498ns  | 34306626ns  | 3846524858ns  |
| Create all attrs  | 7101530ns  | 74663888ns  | 8245685970ns  |
| Delete all attrs  |  120232ns  |   125293ns  |     453306ns  |
+-------------------+------------+-------------+---------------+

Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>
@sachinholla
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 103 in repo sonic-net/sonic-gnmi

Copy link
Copy Markdown
Contributor

@tomek-US tomek-US left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anand-kumar-subramanian anand-kumar-subramanian merged commit a9baca0 into sonic-net:master May 22, 2023
@sachinholla sachinholla deleted the ygotdiff branch June 18, 2023 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants