CVL Enhancement and Fixes #110
Merged
Conversation
This file contains 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
faraazbrcm
force-pushed
the
master
branch
3 times, most recently
from
96d0727 to
95ce88c
Compare
sachinholla
reviewed
Oct 12, 2023
cvl/cvl_test.go
Outdated
| func init() { | ||
| configDb, _ = db.NewDB(db.Options{ | ||
| DBNo: db.ConfigDB, | ||
| InitIndicator: "CONFIG_DB_INITIALIZED", |
There was a problem hiding this comment.
Remove this "InitIndicator" option to fix pipeline test errors. It would be a fresh installation of redis on the build VM and CONFIG_DB_INITIALIZED would not have been set.
Also, do not ignore error returned by db.NewDB(). Add a panic.
faraazbrcm
force-pushed
the
master
branch
5 times, most recently
from
ebba16e to
45fc6e3
Compare
faraazbrcm
force-pushed
the
master
branch
4 times, most recently
from
f047993 to
0f7679d
Compare
faraazbrcm
force-pushed
the
master
branch
3 times, most recently
from
84d225c to
4d27cec
Compare
|
After initial clone and make, I notice a format-check error. |
I dont. see any issue. just cloned using below steps and applied PR content on top of it. I see build went through. please provide the steps you followed when u clone and compiled. Go version needs to be go1.15.15. This is what community uses. please check your go version. My Environment My build log |
kwangsuk
reviewed
Jan 8, 2024
kwangsuk
reviewed
Jan 8, 2024
kwangsuk
reviewed
Jan 8, 2024
kwangsuk
reviewed
Jan 8, 2024
kwangsuk
reviewed
Jan 8, 2024
kwangsuk
reviewed
Jan 8, 2024
anand-kumar-subramanian
force-pushed
the
master
branch
from
0369626 to
cc8bc6f
Compare
kwangsuk
reviewed
Jan 9, 2024
1. Enhancement - Support TABLE keys modelled as container
2. Enhancement - DBAL CountKeysPattern API
3. Optimisation: removing libyang syntax checks for Full entry delete
4. Enhancement - CVL support for SONiC YANG table with distinct YANG
4.1 Enhancement - For multi-list tables correct Redis pattern and sep
are matched and picked.
4.2 Fix - The cvl code to build table expression based on YANG list name
instead of table name.
4.3 Fix - CVL maintains tableInfo using a LIST name, fixed the
inconsistent
4.4 Enhancement - code to lookup using list name instead of table name.
4.5 Fix - recognition of table and fields if the table contains mutiple
lists and if table name as prefix is not present in any of the list
name
5. Optimisation: removing extra checks in CVL for non existing fields
for update operation
6. Fixes: CVL does not honours "not equal" operator in must expression
predicate
7. Fixes: clear depDataCache along with semantic cache, when CVL
validation completes for clen state.
```text
CVL maintains a separate cache to check if dependent data is already
to
the semantic cache. If the entry is present in this cache, the
semantic
cache is not updated.
When we clear clear the semantic cache after every
ValidateEditConfig
we also need to clear depDataCache.
```
8. Fixes: clear Xpath engine cache.
```text
CVL maintains a seesion level cache for semantic validation, this
cache generates entry for table each time they are operated on.
As a result this cache contains duplicate entries for a table
and in some cases it creates a stale entries for leaf, if they are
deleted in the Latest request.
```
9 Fixes: stale entries in semantic documents
```text
CVL maintains a cache of semantic document at the session level.
If Delete for an entry is successful, the cache
still contains a deleted entry, this is resulting in must expression
failure for count cases, the logic in count() considers this entry
in to
consideration. Therefore cache is cleared for the deleted entry.
- Also modified addYangDataForMustExp() to avoid using global cache
and use CVL wrapper for DBAL.
- Modified fetchTableDataToTmpCache() to use CVL wrapper for DBAL's
pipeline.
```
10: Fixes: Incorrect match for depdata in checkDeleteInRequestCache
```text
For find in request key, case - T2*|K1. If the non-delete
configuration exists
for dependent data in the Requestcache, its getting matched, and cvl
considers
its already present in deleteCache and therefore ignores validation.
modified the condition to match only if entry is deleted.
Further enhanced the code for find in request hash-field case -
T2*|K2:{H1: K1}
to match correcly for UPDATE cases. Also in case of leaflist.
```
11: Enhancement: Use DBAL's CVL wrapper for pipeline - All pipelines
were modified use CVL's DB wrapper.
12: Enhancement: Use CVL's DB wrapper in validateLeafref and fixes for
multi-key table instance
13: Fixes - handle incremental build and stale files removal - Fixed yin
generator
```text
Tool does not write if the file is not modified and writes only when
yin modified or added as new.
Also it removes any stale yin files (if present)
```
14. Enhancement: min-elements support for leaf-list
15. Enhancement - Advanced search functions in cvl DBAccess
16. Go test enhancements
17. Update the test YANGs with more complex cases.
18. Enhancement: changes to the count and filter lua scripts to check
both the db and candidate config data; modified the count and lookup
19. Enhancement: Added Pipe query support for HGetAll, HGet, HMGet;
NewValidationSession, cvlDBAccess changes, removing
validatedDataQuery
20. Fixes: Modify the session cache type from interface to
map[string]interface in the custom validations, and used specific
cache key name to access its own cache to avoid overwriting the same
cache object by other module validation code
21. Enhancement: Added DB access adapter like APIs for redis APIs in CVL
layer to access the DB access methods from the CVL layer.
22. Fixes: Added support for NULL field in the Keys, Exists API, fixed
merging issue in HGetAll, HGet, HMGet in case of delete operation
type
23. Fixes: Made changes to handle the redis.Nil error returned by redis
client API.
24. Fixes: Leafref check should check for all DELETE and CREATE of
referred configs in request cache.
25. Fixes: Removed DB ping call in CVL whenever a new DB client is
created.
26. Enhacement: Debugging and log enhancement.
27. Enhancement: CVL logs to appear in Rest-server logs.
28. Improved CVL performance for bulk configs like ACL rules creation.
```text
For evaluation of leafref, When and Must expressions, CVL queries
dependent data and merge with config data and prepares im-memory XML
to
be evaluated by XPath engine. The 'addDependentData" call was adding
same dependent data multiple whenever it has to evaluate leafref,
when
or must expression. When multiple ACL Rules within same Acl-set are
configured in single request, CVL was adding Acl-set multiple times
dues
to which XML size becomes too huge and XPath engines takes long to
evaluate. For 750 Acl rules creation there is 95% improvement in
performance.
```
29. Added support for multiple custom validation callbacks per node in
CVL
30. Fixes: or Replace operations, before performing semantic validation
on Update request, delete fields from yang tree that are provided
with Delete request.
31. Enhancement: Added new extension to define dependency between
tables.
```text
This new extension 'dependent-on <list-name>' has to be used when
dependencies between tables can't
be defined using leafref.
This has to be added under list.
It should not be used under container or leaf/leaf-list nodes.
Only one list-name can be added to this extension.
```
32. Fixes: When mandatory node is getting deleted during cascade-delete,
entire entry to be deleted. So find dependent entries in this entry
and mark them too for delete.
33. Fixed memory leak observed in CVL CGo code that uses libyang third
party library for Syntax validation.
Co-authored-by: Mohammed Faraaz C <[email protected]>
Co-authored-by: Sachin Holla <[email protected]>
Co-authored-by: Balachandar Mani <[email protected]>
Co-authored-by: Mayank Maheshwari <[email protected]>
Co-authored-by: Shyam Mohan <[email protected]>
1. Enhancement - Support TABLE keys modelled as container
2. Enhancement - DBAL CountKeysPattern API
3. Optimisation: removing libyang syntax checks for Full entry delete
4. Enhancement - CVL support for SONiC YANG table with distinct YANG
4.1 Enhancement - For multi-list tables correct Redis pattern and sep
are matched and picked.
4.2 Fix - The cvl code to build table expression based on YANG list name
instead of table name.
4.3 Fix - CVL maintains tableInfo using a LIST name, fixed the
inconsistent
4.4 Enhancement - code to lookup using list name instead of table name.
4.5 Fix - recognition of table and fields if the table contains mutiple
lists and if table name as prefix is not present in any of the list
name
5. Optimisation: removing extra checks in CVL for non existing fields
for update operation
6. Fixes: CVL does not honours "not equal" operator in must expression
predicate
7. Fixes: clear depDataCache along with semantic cache, when CVL
validation completes for clen state.
```text
CVL maintains a separate cache to check if dependent data is already
to
the semantic cache. If the entry is present in this cache, the
semantic
cache is not updated.
When we clear clear the semantic cache after every
ValidateEditConfig
we also need to clear depDataCache.
```
8. Fixes: clear Xpath engine cache.
```text
CVL maintains a seesion level cache for semantic validation, this
cache generates entry for table each time they are operated on.
As a result this cache contains duplicate entries for a table
and in some cases it creates a stale entries for leaf, if they are
deleted in the Latest request.
```
9 Fixes: stale entries in semantic documents
```text
CVL maintains a cache of semantic document at the session level.
If Delete for an entry is successful, the cache
still contains a deleted entry, this is resulting in must expression
failure for count cases, the logic in count() considers this entry
in to
consideration. Therefore cache is cleared for the deleted entry.
- Also modified addYangDataForMustExp() to avoid using global cache
and use CVL wrapper for DBAL.
- Modified fetchTableDataToTmpCache() to use CVL wrapper for DBAL's
pipeline.
```
10: Fixes: Incorrect match for depdata in checkDeleteInRequestCache
```text
For find in request key, case - T2*|K1. If the non-delete
configuration exists
for dependent data in the Requestcache, its getting matched, and cvl
considers
its already present in deleteCache and therefore ignores validation.
modified the condition to match only if entry is deleted.
Further enhanced the code for find in request hash-field case -
T2*|K2:{H1: K1}
to match correcly for UPDATE cases. Also in case of leaflist.
```
11: Enhancement: Use DBAL's CVL wrapper for pipeline - All pipelines
were modified use CVL's DB wrapper.
12: Enhancement: Use CVL's DB wrapper in validateLeafref and fixes for
multi-key table instance
13: Fixes - handle incremental build and stale files removal - Fixed yin
generator
```text
Tool does not write if the file is not modified and writes only when
yin modified or added as new.
Also it removes any stale yin files (if present)
```
14. Enhancement: min-elements support for leaf-list
15. Enhancement - Advanced search functions in cvl DBAccess
16. Go test enhancements
17. Update the test YANGs with more complex cases.
18. Enhancement: changes to the count and filter lua scripts to check
both the db and candidate config data; modified the count and lookup
19. Enhancement: Added Pipe query support for HGetAll, HGet, HMGet;
NewValidationSession, cvlDBAccess changes, removing
validatedDataQuery
20. Fixes: Modify the session cache type from interface to
map[string]interface in the custom validations, and used specific
cache key name to access its own cache to avoid overwriting the same
cache object by other module validation code
21. Enhancement: Added DB access adapter like APIs for redis APIs in CVL
layer to access the DB access methods from the CVL layer.
22. Fixes: Added support for NULL field in the Keys, Exists API, fixed
merging issue in HGetAll, HGet, HMGet in case of delete operation
type
23. Fixes: Made changes to handle the redis.Nil error returned by redis
client API.
24. Fixes: Leafref check should check for all DELETE and CREATE of
referred configs in request cache.
25. Fixes: Removed DB ping call in CVL whenever a new DB client is
created.
26. Enhacement: Debugging and log enhancement.
27. Enhancement: CVL logs to appear in Rest-server logs.
28. Improved CVL performance for bulk configs like ACL rules creation.
```text
For evaluation of leafref, When and Must expressions, CVL queries
dependent data and merge with config data and prepares im-memory XML
to
be evaluated by XPath engine. The 'addDependentData" call was adding
same dependent data multiple whenever it has to evaluate leafref,
when
or must expression. When multiple ACL Rules within same Acl-set are
configured in single request, CVL was adding Acl-set multiple times
dues
to which XML size becomes too huge and XPath engines takes long to
evaluate. For 750 Acl rules creation there is 95% improvement in
performance.
```
29. Added support for multiple custom validation callbacks per node in
CVL
30. Fixes: or Replace operations, before performing semantic validation
on Update request, delete fields from yang tree that are provided
with Delete request.
31. Enhancement: Added new extension to define dependency between
tables.
```text
This new extension 'dependent-on <list-name>' has to be used when
dependencies between tables can't
be defined using leafref.
This has to be added under list.
It should not be used under container or leaf/leaf-list nodes.
Only one list-name can be added to this extension.
```
32. Fixes: When mandatory node is getting deleted during cascade-delete,
entire entry to be deleted. So find dependent entries in this entry
and mark them too for delete.
33. Fixed memory leak observed in CVL CGo code that uses libyang third
party library for Syntax validation.
Co-authored-by: Mohammed Faraaz C <[email protected]>
Co-authored-by: Sachin Holla <[email protected]>
Co-authored-by: Balachandar Mani <[email protected]>
Co-authored-by: Mayank Maheshwari <[email protected]>
Co-authored-by: Shyam Mohan <[email protected]>
faraazbrcm
force-pushed
the
master
branch
2 times, most recently
from
f44754f to
43242e2
Compare
1. sortDepTables 2. GetOrderedDepTables 3. GetOrderedTables Also fixed following issues in the above APIs 1. Missing Nodes(toposort) when Table has multiple lists. 2. Inconsistent comparison (few places list names where compared with table name) 3. Inconsistent I/O. These functions were designed to work on Lists but it accepted table names and due to which inconsistent results were observed. Fixed the behavior the I/O is always TABLE NAME but internally it gets converted to LIST and processed. It is done because there is a possibility of having an dependency between TABLE LIST. 4. Optimise the code. Co-authored-by: Mohammed Faraaz C <[email protected]>
kwangsuk
approved these changes
Jan 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CVL Infra Enhancments and Fixes
4.1 Enhancement - For multi-list tables correct Redis pattern and sep
are matched and picked.
4.2 Fix - The cvl code to build table expression based on YANG list name
instead of table name.
4.3 Fix - CVL maintains tableInfo using a LIST name, fixed the
inconsistent
4.4 Enhancement - code to lookup using list name instead of table name.
4.5 Fix - recognition of table and fields if the table contains mutiple
lists and if table name as prefix is not present in any of the list
name
for update operation
predicate
validation completes for clen state.
9 Fixes: stale entries in semantic documents
10: Fixes: Incorrect match for depdata in checkDeleteInRequestCache
11: Enhancement: Use DBAL's CVL wrapper for pipeline - All pipelines
were modified use CVL's DB wrapper.
12: Enhancement: Use CVL's DB wrapper in validateLeafref and fixes for
multi-key table instance
13: Fixes - handle incremental build and stale files removal - Fixed yin
generator
both the db and candidate config data; modified the count and lookup
NewValidationSession, cvlDBAccess changes, removing
validatedDataQuery
map[string]interface in the custom validations, and used specific
cache key name to access its own cache to avoid overwriting the same
cache object by other module validation code
layer to access the DB access methods from the CVL layer.
merging issue in HGetAll, HGet, HMGet in case of delete operation
type
client API.
referred configs in request cache.
created.
CVL
on Update request, delete fields from yang tree that are provided
with Delete request.
tables.
entire entry to be deleted. So find dependent entries in this entry
and mark them too for delete.
party library for Syntax validation.
Co-authored-by: Mohammed Faraaz C [email protected]
Co-authored-by: Sachin Holla [email protected]
Co-authored-by: Balachandar Mani [email protected]
Co-authored-by: Mayank Maheshwari [email protected]
Co-authored-by: Shyam Mohan [email protected]