Git Product home page Git Product logo

Comments (7)

michalvasko avatar michalvasko commented on July 22, 2024 1

I tried to use ordered lists, but I encountered some issues obtaining indices from the values; maybe that's better and I just didn't figure it out.

You are most likely right regarding the performance problems origins and yes, if you are fine with the index being represented by the position of the instance, user-ordered lists may be the solution. They are a but more difficult to work with but it should all work and what does not can be fixed. However, getting the index of an instance is a linear operation because they are not stored anywhere. It seems it would not be trivial to get the optimal design.

Potential bug

Sorry about that, I have tested it only briefly so this slipped, should be fixed. Also, the other error for * operational datastore plugin should no longer occur.

from sysrepo.

michalvasko avatar michalvasko commented on July 22, 2024

write to a JSON file in about 1-2ms, but if I write the data into sysrepo, it takes 200ms to call sr_apply_changes because of the validation.

That really seems like a huge difference but possible in case there are lots of validation tasks.

write to a JSON file in about 1-2ms, but if I write the data into sysrepo, it takes 200ms to call sr_apply_changes because of the validation.

There is a fairly recent feature that allows you to "disable" running datastore which essentially means it always mirrors startup.

I see there is some kind of default JSON plugin, but I don't know how to use/enable it.

This plugin has replaced the LYB plugin and like you said, it is the default so there is nothing to enable. Unless you configure a module to use a different plugin, this one is always used.

I have read over the Datastore Plugin API, but I don't understand where/how to call these functions.

It is a plugin meaning the functions are called automatically by sysrepo, similarly to any *.so library.

I would be willing to help improve the docs page once I figure out how it actually works.

That is appreciated but I also think you lack a bit some basic understanding of Linux concepts.

So my question is what step did I miss to get these callbacks to actually fire?

You need to install a module that will be using your plugin. Look at sysrepoctl -h and the -m parameter.

Does it make sense to use a JSON Datastore Plugin to try to improve performance in my case? For CLI/Web GUI interactions, I can save directly to the JSON file (self-validated, fast).

This is not really the intended use but it should work perfectly fine. Meaning writing your own JSON-based plugin instead of using the default one. Just make sure you implement every callback correctly, we are working on some other datastore plugins and have tried to document every requirement as we encountered it but there are quite a lot of callbacks by now and they are not trivial to implement. Nevertheless, you wanting to use JSON files simplifies it because you can most likely reuse lots of code form the default JSON callbacks.

from sysrepo.

nullromo avatar nullromo commented on July 22, 2024

Hi Michal, thank you for the response.

possible in case there are lots of validation tasks.

It's also possible I wrote the must statements in a non-optimal way...

fairly recent feature

I will certainly try this out! Sounds like exactly what I need.

That is appreciated but I also think you lack a bit some basic understanding of Linux concepts.

I'm just talking about something like #3256. Just to make it more clear how to set it up. A small pointer in the docs like this goes a long way.

Look at sysrepoctl -h and the -m parameter

Perfect. Missed this before.

Meaning writing your own JSON-based plugin

I will try this out, thanks.

from sysrepo.

nullromo avatar nullromo commented on July 22, 2024

Sorry for making this comment so long, but there's a lot to say.

There is a fairly recent feature

For anyone else that comes across this issue, here's an explanation of how to use this.

EDIT: this is outdated. See #3257

If you call sysrepoctl --install without specifying --module-plugin, then that module will be installed using the default datastore plugins. It will behave normally. The name of the default JSON plugin that sysrepo uses is JSON DS file for every datastore except notification. For notification, it is called JSON notif.

If you want to set the name of the running datastore's plugin for the module to NULL, the way to do that is to specify all the other datastores and leave the running datastore blank.

So for example, you could use the following command:

sysrepoctl --install <my-module> \
           --module-plugin "startup:JSON DS file" \
           --module-plugin "candidate:JSON DS file" \
           --module-plugin "operational:JSON DS file" \
           --module-plugin "factory-default:JSON DS file" \
           --module-plugin "notification:JSON notif"

By specifying plugins for no datastores, you get the default. By specifying the default datastores for everything except the running datastore, you get the running datastore actually set to NULL.

Another important note

Remember that this is per-module. If you want to actually lock the startup and running datastore together, then you need to include all the --module-plugin arguments from above every time you install any module. Otherwise, you could end up with 1 module using separate startup/running data (the default) and another module using unified data.

Suggestion

EDIT: "running:" works now as of #3257

@michalvasko I think perhaps allowing --module-plugin "running:" would be a better solution, i.e. something similar to this:

+ if(strlen(ptr) == 1) {
+     module_ds->plugin_name[module_ds_idx] = NULL;
+ } else {
- module_ds->plugin_name[module_ds_idx] = ptr + 1;
+     module_ds->plugin_name[module_ds_idx] = ptr + 1;
+ }

Problem

After installing all my modules with the above --module-plugin arguments, I get this issue:

$ sudo sysrepocfg --export --datastore operational -v4
[INF] Connection 125 created.
[INF] Session 197 (user "root", CID 125) created.
[ERR] Datastore plugin "*" not found.
sysrepocfg error: Getting data failed (Item not found)

I don't know how it got "*" as the datastore plugin. In addition, I looked under the sysrepo repository file data/sysrepo.startup and I found that all the plugins for operational were listed as "JSON DS file", as expected (see below for example)

{
    "name": "yang",
    "revision": "2022-06-16",
    "plugin": [
        { "datastore": "ietf-datastores:startup", "name": "JSON DS file" },
        { "datastore": "ietf-datastores:running", "name": "JSON DS file" },
        { "datastore": "ietf-datastores:candidate", "name": "JSON DS file" },
        { "datastore": "ietf-datastores:operational", "name": "JSON DS file" },
        { "datastore": "ietf-factory-default:factory-default", "name": "JSON DS file" },
        { "datastore": "notification", "name": "JSON notif" }
    ],
    "deps": {}
}

I tried this on the current master branch.

Question

I have another question, which is how do I do this process for the internal sysrepo modules? For example, I cannot uninstall ietf-netconf-acm because it is internal.

$ sudo sysrepoctl --uninstall --module ietf-netconf-acm -v4
[INF] Connection 5 created.
[ERR] Internal module "ietf-netconf-acm" cannot be uninstalled.
sysrepoctl error: Failed to uninstall modules (Invalid argument)

It appears you cannot install a module that is already installed:

$ sudo sysrepoctl --install yang/[email protected] -m "startup:foo" -v4
[INF] Connection 14 created.
[WRN] Module "ietf-netconf-acm" is already in sysrepo.

so how would I set the running datastore to NULL for an internal module?

from sysrepo.

michalvasko avatar michalvasko commented on July 22, 2024

It's also possible I wrote the must statements in a non-optimal way...

It is true that some statements are faster to evaluate than others despite having the same functionality. Also, libyang is able to specially optimize some expressions so if you think it worth it, you can post the module/must expressions and I can provide feedback.

I'm just talking about something like #3256. Just to make it more clear how to set it up. A small pointer in the docs like this goes a long way.

I see. Well, to some extent this is intentional because writing your own datastore plugin is not trivial and it being extensively documented may give the impression it is. But I think with basic knowledge of sysrepo and looking at the default plugin, it should be possible without much trouble (I know about a few implementations).

I think perhaps allowing --module-plugin "running:" would be a better solution, i.e. something similar to this:

Seems reasonable and if this is all that is required (in addition to docs update), you can provide a PR (but to devel please).

I looked under the sysrepo repository file data/sysrepo.startup and I found that all the plugins for operational

All the plugins, not just operational? In any case, I would be interested in reproducing this so I can fix it, if there is something to fix. You should definitely get an error sooner than when exporting data.

how would I set the running datastore to NULL for an internal module?

Not possible currently, but I suppose it should be supported, I will come up with a solution.

EDIT: Implemented now when you can set all the modules with disabled running DS in a cmake variable.

from sysrepo.

nullromo avatar nullromo commented on July 22, 2024

must statemets

I fully don't expect you to help with my YANG model, since it's my issue to figure out, but if you're willing to take a look and point out anything I'm sure it would be helpful. I had to remove the description and error-message nodes and the comments, and rename most of the nodes to post it publicly.

Minimal YANG snippet to demonstrate easier
typedef class {
    type enumeration {
        enum A;
        enum E;
        enum F;
        enum G;
        enum M;
        enum P;
        enum S;
        enum T;
        enum X;
    }
}

grouping channel {
    leaf channel {
        type uint16 {
            range '1..max';
        }
    }
}

list ccc {
    key 'class ccc-key';
    leaf class {
        type class;
    }
    leaf ccc-key {
        type uint8;
    }

    // style 1 must statement that ensures the right number of ggg elements
    must "(class != 'E') or (count(ggg) = channels)";
    list ggg {
        when "../class = 'E'";
        key channel;
        uses channel;
        // style 2 must statement that ensures the indices go from 1 to N
        must "current()/channel = 1 or ../ggg/channel[. = current()/channel - 1]";
        leaf value {
            type uint8;
            mandatory true;
        }
    }
    leaf channels {
        when "../class = 'A' or " +
             "../class = 'E' or " +
             "../class = 'F' or " +
             "../class = 'T'";
        type uint16 {
            range '1..max';
        }
        mandatory true;
    }
}
Full YANG snippet of relevant section for reference purposes
typedef class {
    type enumeration {
        enum A;
        enum E;
        enum F;
        enum G;
        enum M;
        enum P;
        enum S;
        enum T;
        enum X;
    }
}

grouping channel {
    leaf channel {
        type uint16 {
            range '1..max';
        }
    }
}

container tree {
    list aaa {
        key aaa-key;
        leaf aaa-key {
            type uint8;
        }
        list bbb {
            key bbb-key;
            leaf bbb-key {
                type uint8;
            }
            must "current()/bbb-key = 1 or ../bbb/bbb-key[. = current()/bbb-key - 1]";
            list ccc {
                key 'class ccc-key';
                leaf class {
                    type class;
                }
                leaf ccc-key {
                    type uint8;
                }
                must "(class != 'A' and class != 'E' and class != 'F' and class != 'T') or " +
                     "(count(ddd/m) = channels and count(ddd/n) = channels)";
                must "(class != 'A' and class != 'E' and class != 'F' and class != 'T') or " +
                     "(count(eee) = channels)";
                must "(class != 'M' and class != 'X') or " +
                     "(count(ddd/m) = dimensions/m and count(ddd/n) = dimensions/n)";
                must "(class != 'M' and class != 'X') or " +
                     "(count(eee) = dimensions/m)";
                must "(class != 'P' and class != 'S') or " +
                     "(count(ddd/m) = 1 and count(ddd/n) = 1)";
                must "(class != 'A' and class != 'E') or " +
                     "(count(fff) = channels)";
                must "(class != 'E') or " +
                     "(count(ggg) = channels)";
                must "(class != 'E') or " +
                     "(count(hhh) = channels)";
                must "(class != 'E' and class != 'T') or " +
                     "(count(iii) = channels)";
                must "(class != 'E' and class != 'T') or " +
                     "(count(jjj) = channels)";
                must "(class != 'X') or " +
                     "(count(mmm) = dimensions/m)";
                container ddd {
                    list m {
                        key channel;
                        uses channel;
                        must "current()/channel = 1 or ../m/channel[. = current()/channel - 1]";
                        leaf value {
                            type string;
                            mandatory true;
                        }
                    }
                    list n {
                        key channel;
                        uses channel;
                        must "current()/channel = 1 or ../n/channel[. = current()/channel - 1]";
                        leaf value {
                            type string;
                            mandatory true;
                        }
                    }
                }
                list fff {
                    when "../class = 'A' or ../class = 'E'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../fff/channel[. = current()/channel - 1]";
                    leaf value {
                        type double;
                        must "current() >= 0 and current() <= ../../max-fff";
                        mandatory true;
                    }
                }
                list eee {
                    when "../class = 'A' or " +
                         "../class = 'E' or " +
                         "../class = 'F' or " +
                         "../class = 'M' or " +
                         "../class = 'T' or " +
                         "../class = 'X'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../eee/channel[. = current()/channel - 1]";
                    leaf value {
                        type double;
                        must "(../../class = 'F' or current() = ../../allowed-eee)";
                        must "(../../class != 'F' or " +
                             "(current() >= ../../eee-range/min and " +
                              "current() <= ../../eee-range/max)" +
                             ")";
                        mandatory true;
                    }
                }
                list ggg {
                    when "../class = 'E'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../ggg/channel[. = current()/channel - 1]";
                    leaf value {
                        type uint8 {
                            range '0..1';
                        }
                        mandatory true;
                    }
                }
                list hhh {
                    when "../class = 'E'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../hhh/channel[. = current()/channel - 1]";
                    leaf value {
                        type double;
                        must "current() >= ../../hhh-range/min and current() <= ../../hhh-range/max";
                        mandatory true;
                    }
                }
                list jjj {
                    when "../class = 'E' or ../class = 'T'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../jjj/channel[. = current()/channel - 1]";
                    leaf value {
                        type uint8 {
                            range '0..1';
                        }
                        mandatory true;
                    }
                }
                list iii {
                    when "../class = 'E' or ../class = 'T'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../iii/channel[. = current()/channel - 1]";
                    leaf value {
                        type double;
                        mandatory true;
                    }
                }
                leaf ooo {
                    when "../class = 'M'";
                    type uint8;
                    must "current() >= 1 and current() <= ../dimensions/m";
                    mandatory true;
                }
                leaf ppp {
                    when "../class = 'M'";
                    type uint8;
                    must "current() >= 0 and current() <= ../dimensions/n";
                    mandatory true;
                }
                leaf nnn {
                    when "../class = 'P' or ../class = 'S'";
                    type uint8;
                    must "current() >= ../nnn-range/min and current() <= ../nnn-range/max";
                    mandatory true;
                }
                list mmm {
                    when "../class = 'X'";
                    key channel;
                    uses channel;
                    must "current()/channel = 1 or ../mmm/channel[. = current()/channel - 1]";
                    leaf value {
                        type uint16;
                        must "current() >= 0 and current() <= ../../dimensions/n";
                        must "current() = 0 or not(../../mmm[channel != current()/../channel]/value[. = current()])";
                        mandatory true;
                    }
                }
                leaf channels {
                    when "../class = 'A' or " +
                         "../class = 'E' or " +
                         "../class = 'F' or " +
                         "../class = 'T'";
                    type uint16 {
                        range '1..max';
                    }
                    mandatory true;
                }
                leaf max-fff {
                    when "../class = 'A' or " +
                         "../class = 'E'";
                    type double;
                    mandatory true;
                }
                leaf-list allowed-eee {
                    when "../class = 'A' or " +
                         "../class = 'E' or " +
                         "../class = 'M' or " +
                         "../class = 'T' or " +
                         "../class = 'X'";
                    type double;
                }
                container eee-range {
                    when "../class = 'F'";
                    leaf min {
                        type double;
                        mandatory true;
                    }
                    leaf max {
                        type double;
                        mandatory true;
                    }
                }
                container hhh-range {
                    when "../class = 'E' or " +
                         "../class = 'T'";
                    leaf min {
                        type double;
                        mandatory true;
                    }
                    leaf max {
                        type double;
                        mandatory true;
                    }
                }
                container dimensions {
                    when "../class = 'M' or " +
                         "../class = 'X'";
                    leaf m {
                        type uint16 {
                            range '1..max';
                        }
                        mandatory true;
                    }
                    leaf n {
                        type uint16 {
                            range '1..max';
                        }
                        mandatory true;
                    }
                }
                container nnn-range {
                    when "../class = 'P' or " +
                         "../class = 'S'";
                    leaf min {
                        type uint16;
                        mandatory true;
                    }
                    leaf max {
                        type uint16;
                        mandatory true;
                    }
                }
            }
        }
    }
}

In summary I have a polymorphic list of items, where each item has a class, and that determines which fields it contains. Based on the class, certain nodes will be present due to the when statements. I don't think this part should cause any problems.

There are 2 main styles of must statements used.

  • The first is the stack of them at the top, which restricts the lengths of certain lists. For example, if class is E, then a ggg list will be present and a channels leaf will be present. I need the number of items in the ggg list to equal the channels value. So I use must "(class != 'E') or (count(ggg) = channels)";.
  • The second is the list index restriction, which I think is the performance culprit. Unfortunately due to my application, I need to have each list contain numeric indices. I need to be able to address items by their numerical index and also find the numerical indices of items with certain values. So my lists here are using { channel, value } pairs instead of just values. As you can see, the lists use the channel grouping as a key, and they all have the restriction must "current()/channel = 1 or ../<something>/channel[. = current()/channel - 1]";, which restricts the channel indices such that they have to start at 1 and never skip any numbers. Unfortunately it feels like re-inventing a normal indexed list. I tried to use ordered lists, but I encountered some issues obtaining indices from the values; maybe that's better and I just didn't figure it out.

Anyway, like I said any feedback is appreciated but I don't want to waste your time with this since it's kind of off-topic.

Empty string for NULL datastore plugin

#3257 does 2 things:

  1. allow <datastore>: to be specified to set a datastore plugin to NULL.
  2. allow specifying a single plugin as NULL and still use the defaults for the rest.

There is a small problem with the code style. See the PR for details.

Datastore plugin '*' not found

All the plugins, not just operational?

Yes, all of them. I didn't explain it properly, but this error message occurred ONLY when using the operational datastore. All the other datastores worked fine, and running was successfully tied to startup.

-DINTERNAL_MODULE_DISABLED_RUNNING=* cmake variable

Nice solution. I tried this and looked at /etc/sysrepo/data/sysrepo.startup, and indeed there were no mentions of running anywhere.

Potential bug

I took the following steps:

  1. checkout devel for libyang and libnetconf2, and build + install
  2. checkout devel for sysrepo, build with
    cmake .. -DCMAKE_BUILD_TYPE=Release -DINTERNAL_MODULE_DATA_PATH=<path-to-xml> -DINTERNAL_MODULE_DISABLED_RUNNING=*
    
    and install.
  3. sudo make sr_clean
  4. sudo sysrepocfg --export -v3
    This is where I got problems. I saw [ERR] Datastore plugin ")" not found. at one point. Later on after various attempts to re-compile, re-clean, delete /etc/sysrepo, etc. I saw " not found.ore plugin ", which is just [ERR] Datastore plugin "\r" not found. (with a carriage return as the plugin name). So it looks to me like the plugin name is printing a bad byte from memory.
  5. Additionally, I tried to install netopeer2 from the current devel branch, but I got the same "not found" message there during the install.
  6. I tried building sysrepo with
    -DINTERNAL_MODULE_DISABLED_RUNNING="ietf-netconf-acm"
    
    and it seemed to work fine.
  7. I tried building sysrepo with
    -DINTERNAL_MODULE_DISABLED_RUNNING="ietf-datastores ietf-factory-default ietf-inet-types ietf-netconf ietf-netconf-acm ietf-netconf-notifications ietf-netconf-with-defaults ietf-origin ietf-yang-library ietf-yang-metadata ietf-yang-schema-mount ietf-yang-structure-ext ietf-yang-types sysrepo-factory-default sysrepo-monitoring sysrepo-plugind yang"
    
    and it had the "not found" error again. (I got this list of modules from looking at sysrepoctl --list after a fresh make sr_clean.)
  8. I tried many combinations of listing out the modules, and I found that the ONLY problematic one from this list was ietf-yang-schema-mount. In other words, building with
    -DINTERNAL_MODULE_DISABLED_RUNNING="ietf-datastores ietf-factory-default ietf-inet-types ietf-netconf ietf-netconf-acm ietf-netconf-notifications ietf-netconf-with-defaults ietf-origin ietf-yang-library ietf-yang-metadata ietf-yang-structure-ext ietf-yang-types sysrepo-factory-default sysrepo-monitoring sysrepo-plugind yang"
    
    worked with no problems.

So in summary, I think there is a problem with using this feature on ietf-yang-schema-mount.

from sysrepo.

nullromo avatar nullromo commented on July 22, 2024

Upon more investigation, I don't think a custom DS plugin is worth doing for my case, because modifying the backing JSON file directly doesn't hook into the sysrepo-plugind plugins anyway, so it becomes way more of a pain to handle subscriptions. At that point, going through sysrepo normally will be the best option.

However, this thread has been very helpful in improving the performance of my application.

  • Disabling the running datastore saved a lot of sync time and makes sense for my application because I don't need it.
  • Compiler optimization resulted in a small improvement.
  • I re-wrote the must statements so they don't need to search through the tree.
    • Where I used to have must "current()/channel = 1 or ../ggg/channel[. = current()/channel - 1]", I now have must "(current()/channel >= 1 and current()/channel <= ../channels)" instead. Same restriction effectively, but faster to evaluate.

As always, thank you Michal

from sysrepo.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.