sysrepo / sysrepo

YANG-based configuration and operational state data store for Unix/Linux applications
http://www.sysrepo.org
BSD 3-Clause "New" or "Revised" License
347 stars 232 forks source link

Recursive loading of mod diff in datastore plugin #3399

Open bala1796 opened 3 weeks ago

bala1796 commented 3 weeks ago

Hi @michalvasko

I'm trying an edit-config operation for a module managed by my datastore plugin where I'm trying to change the value of node 'd'.

The mod_diff parameter that I receive in the callback and print through lyd_print_tree() or lyd_print_all() is shown below.

<a xmlns="xxx" xmlns:yang="urn:ietf:params:xml:ns:yang:1" yang:operation="none">
  <b yang:operation="none">
    <c yang:operation="none">
      <d yang:operation="replace" yang:orig-value="old_val" yang:orig-default="false">new_val</d>
    </c>
  </b>
</a>

This tree looks right.

But when I pass mod_diff to the function xx_load_diff_recursively (same as the one present in ds_mongo.c/ds_redis.c), I expected it to recursively load only the nodes and operations in the tree above. Instead, I see it adding create operations for children of nodes which are not present in the tree.

For example, if node 'a' has children a1, a2, a3 and b,

I see: 'create' operation for a1, a2 and a3 'none' operation for 'b'

But I expected: 'none' for b.

While children a1,a2, a3 exist in the schema, how does lyd_child_no_keys() get a1,a2 and a3 from the mod_diff instead of just 'b'?

michalvasko commented 3 weeks ago

I do not understand what is it you are trying to do, apply the diff on a data tree? There is a special function for that.

bala1796 commented 3 weeks ago

I want to parse the mod_diff tree and send the node xpath, operation and value to another application so that the diff can be applied there. To do this, I'm first recursively loading the diff operations to a custom data structure (similar to srpds_load_diff_recursively() function present in the ds_mongo.c and ds_redis.c file)

michalvasko commented 3 weeks ago

Just use sr_get_change_diff() to get the diff, which you can even serialize (print) and send whenever you want.

bala1796 commented 3 weeks ago

I'm writing a custom datastore plugin. I'm not subscribed to data changes. I have netopeer2-server connected to sysrepo with a SR_CONN_DEFAULT flag. Cache is disabled. Each time I do an edit-config, my datastore plugin's load_cb() is called and the data is retrieved and the store_cb() is then called where I get the mod_diff. I have a problem with parsing this diff tree. When I print out the diff tree with lyd_print_tree(), the printed tree looks right (the output of this is given in my first comment). I was trying to change the value of node 'd' and I see the replace operation for it in the diff. So far, so good.

How do I parse the diff tree as it is? When I manually parse the diff tree in a loop using lyd_child_no_keys, I get to children that are not present in the tree and the operations associated with them are 'create'.

michalvasko commented 1 week ago

This is not a good design because mod_diff may not always be available and is provided mostly to allow for optimized operations. And are you asking about traversing the tree? Parsing means something else. And yes, you will obviously find non-existing nodes with create operation in the diff. Why is that a problem?

bala1796 commented 1 week ago

I understand that this may not be an efficient design, but are there any other problems with this design? Under what conditions will I not be getting the mod_diff? And yes, I mean traversing the tree. Why do I see those non-existing nodes with create operations only when I traverse the tree and not when I'm printing the tree using lyd_print_tree()? I'm not saying its wrong. I understand that sysrepo expects to have the entire module's data upon calling the load_cb(), but is it possible to return only the subset of the module data and expect a mod_diff containing only the diff between the returned data and the edit-config data without it having create operations for non-existing nodes?

I'm trying to design it this way to avoid having a copy of the configuration data in sysrepo. The only place in which we want to hold the data is in our application's memory. That's the reason I'm disabling the connection cache for netopeer2-server and trying to make sysrepo call my load/store callbacks for each netconf operation.

michalvasko commented 1 week ago

Under what conditions will I not be getting the mod_diff?

In short, when it cannot be created because it would mix data from 2 contexts. This can only happen on YANG context changes (adding/removing modules, changing features, ...) so it may not affect you, especially is the module(s) you are using is fairly simple.

Why do I see those non-existing nodes with create operations only when I traverse the tree and not when I'm printing the tree using lyd_print_tree()?

So you are saying that if you print the diff you do not see the created nodes? You should.

I understand that sysrepo expects to have the entire module's data upon calling the load_cb(), but is it possible to return only the subset of the module data and expect a mod_diff containing only the diff between the returned data and the edit-config data without it having create operations for non-existing nodes?

I do not understand what you are saying. For load cb, you need to return all the current data of the module. How exactly do you construct the data is up to you.

I'm trying to design it this way to avoid having a copy of the configuration data in sysrepo. The only place in which we want to hold the data is in our application's memory.

That is perfectly fine, as long as you have all the required information (data) required by sysrepo available in the application, you do not have to store anything additional anywhere.