diff --git a/CHANGELOG.md b/CHANGELOG.md index c6eba715..5f929ddb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,15 @@ * Two config options control: * CLICON_XML_CHANGELOG enables the yang changelog feature * CLICON_XML_CHANGELOG_FILE where the changelog resides +* Optimization work + * Improved performance of validation of (large) lists + * A scaling of [large lists](doc/scaling) report is added + * New xmldb_get1() returning actual cache - not a copy. This has lead to some householding instead of just deleting the copy + * xml_diff rewritten to work linearly instead of O(2) + * New xml_insert function using tree search. The new code uses this in insertion xmldb_put and defaults. (Note previous xml_insert renamed to xml_wrap_all) + * A yang type regex cache added, this helps the performance by avoiding re-running the `regcomp` command on every iteration. + * An XML namespace cache added (see `xml2ns()`) + * Better performance of XML whitespace parsing/scanning. ### API changes on existing features (you may need to change your code) @@ -102,12 +111,7 @@ ### Minor changes -* A scaling of [large lists](doc/scaling) report is added * A new "hello world" example is added -* Optimized validation of large lists - * New xmldb_get1() returning actual cache - not a copy. This has lead to some householding instead of just deleting the copy - * xml_diff rewritten to work linearly instead of O(2) - * New xml_insert function using tree search. The new code uses this in insertion xmldb_put and defaults. (Note previous xml_insert renamed to xml_wrap_all) * Experimental customized error output strings, see [lib/clixon/clixon_err_string.h] * Empty leaf values, eg are now checked at validation. * Empty values were skipped in validation. diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 9d02568b..658447e8 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -694,8 +694,9 @@ main(int argc, /* Call backend plugin_start with user -- options */ if (clixon_plugin_start(h) < 0) goto done; + /* -1 option to run only once */ if (once) - goto done; + goto ok; /* Daemonize and initiate logging. Note error is initiated here to make demonized errors OK. Before this stage, errors are logged on stderr @@ -733,6 +734,7 @@ main(int argc, goto done; if (event_loop() < 0) goto done; + ok: retval = 0; done: if (cbret) diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index 34102e7a..367ed627 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -260,7 +260,7 @@ startup_extraxml(clicon_handle h, goto done; if (ret == 0) goto fail; - if (xt==NULL /* || xml_child_nr(xt)==0 */ ) /* This gives SEGV in test_feature */ + if (xt==NULL || xml_child_nr(xt)==0) goto ok; /* Write (potentially modified) xml tree xt back to tmp */ diff --git a/doc/scaling/large-lists.md b/doc/scaling/large-lists.md index 229561ab..35424dc0 100644 --- a/doc/scaling/large-lists.md +++ b/doc/scaling/large-lists.md @@ -12,14 +12,14 @@ Olof Hagsand, 2019-04-17 ## 1. Background -CIixon can handle large configurations. Here, large number of elements +Clixon can handle large configurations. Here, large number of elements in a "flat" list is presented. There are other scaling usecases, such as large configuratin "depth", large number of requesting clients, etc. Thanks to [Netgate](www.netgate.com) for supporting this work. -## 2.Overview +## 2. Overview The basic case is a large list, according to the following Yang specification: ``` diff --git a/doc/startup.md b/doc/startup.md index 921c0072..d9937cc0 100644 --- a/doc/startup.md +++ b/doc/startup.md @@ -254,7 +254,8 @@ When the startup process is completed, a startup status is set and is accessible If the startup fails, the backend looks for a `failsafe` configuration in `CLICON_XMLDB_DIR/failsafe_db`. If such a config is not found, the -backend terminates. +backend terminates. In this mode, running and startup mode should be +unchanged. If the failsafe is found, the failsafe config is loaded and committed into the running db. @@ -405,6 +406,7 @@ running |--------+------------> GOTO EXTRA XML ### Running mode +On failure, running is restored to initial state ``` running ----+ |----------+--------> GOTO EXTRA XML \ copy parse validate OK / commit @@ -420,7 +422,7 @@ running |--------+------------> GOTO EXTRA XML startup -------+--+-------+------------+ ``` -### Failure +### Failure if failsafe ``` failsafe ----------------------+ reset \ commit diff --git a/example/Makefile.in b/example/Makefile.in index 90a0aa19..3c9a2a89 100644 --- a/example/Makefile.in +++ b/example/Makefile.in @@ -40,7 +40,7 @@ LIBS = @LIBS@ SHELL = /bin/sh -SUBDIRS = main +SUBDIRS = main hello .PHONY: all clean depend install $(SUBDIRS) diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c index 149cf49d..278a0023 100644 --- a/lib/src/clixon_datastore.c +++ b/lib/src/clixon_datastore.c @@ -188,14 +188,15 @@ xmldb_copy(clicon_handle h, int retval = -1; char *fromfile = NULL; char *tofile = NULL; - db_elmnt *de1 = NULL; - db_elmnt *de2 = NULL; + db_elmnt *de1 = NULL; /* from */ + db_elmnt *de2 = NULL; /* to */ db_elmnt de0 = {0,}; - cxobj *x1 = NULL; - cxobj *x2 = NULL; + cxobj *x1 = NULL; /* from */ + cxobj *x2 = NULL; /* to */ /* XXX lock */ if (clicon_option_bool(h, "CLICON_XMLDB_CACHE")){ + /* Copy in-memory cache */ /* 1. "to" xml tree in x1 */ if ((de1 = clicon_db_elmnt_get(h, from)) != NULL) x1 = de1->de_xml; @@ -208,7 +209,7 @@ xmldb_copy(clicon_handle h, xml_free(x2); x2 = NULL; } - else if (x2 == NULL){ /* create x2 and copy x1 to it */ + else if (x2 == NULL){ /* create x2 and copy from x1 */ if ((x2 = xml_new(xml_name(x1), NULL, xml_spec(x1))) == NULL) goto done; if (xml_copy(x1, x2) < 0) @@ -221,12 +222,13 @@ xmldb_copy(clicon_handle h, if (xml_copy(x1, x2) < 0) goto done; } - if (x1 || x2){ - if (de2) - de0 = *de2; - de0.de_xml = x2; /* The new tree */ - clicon_db_elmnt_set(h, to, &de0); - } + /* always set cache although not strictly necessary in case 1 + * above, but logic gets complicated due to differences with + * de and de->de_xml */ + if (de2) + de0 = *de2; + de0.de_xml = x2; /* The new tree */ + clicon_db_elmnt_set(h, to, &de0); } /* Copy the files themselves (above only in-memory cache) */ if (xmldb_db2file(h, from, &fromfile) < 0) diff --git a/test/lib.sh b/test/lib.sh index 61769b4c..f2a9d32c 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -201,6 +201,7 @@ new(){ # - expected command return value (0 if OK) # - expected stdout outcome, # - expected2 stdout outcome, +# Example: expectfn "$clixon_cli -1 -f $cfg show conf cli" 0 "^$" expectfn(){ cmd=$1 retval=$2 diff --git a/test/test_perf.sh b/test/test_perf.sh index e3513a52..e0fe91af 100755 --- a/test/test_perf.sh +++ b/test/test_perf.sh @@ -5,7 +5,7 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi # Number of list/leaf-list entries in file -: ${perfnr:=2000} +: ${perfnr:=10000} # Number of requests made get/put : ${perfreq:=100} @@ -50,9 +50,36 @@ cat < $cfg false $dir false + example + /usr/local/lib/example/cli + /usr/local/lib/example/clispec + 1 + VARS + 0 EOF +# Try startup mode w startup +for mode in startup running; do + file=$dir/${mode}_db + sudo touch $file + sudo chmod 666 $file + new "generate large startup config ($file) with $perfnr list entries in mode $mode" + echo -n "" > $file + for (( i=0; i<$perfnr; i++ )); do + echo -n "$i$i" >> $file + done + echo "" >> $file + + new "Startup backend once -s $mode -f $cfg -y $fyang" + # Cannot use start_backend here due to expected error case + time sudo $clixon_backend -F1 -D $DBG -s $mode -f $cfg -y $fyang # 2> /dev/null +done + +new "Startup backend once -s $mode -f $cfg -y $fyang" +# Cannot use start_backend here due to expected error case +time sudo $clixon_backend -F1 -D $DBG -s $mode -f $cfg -y $fyang # 2> /dev/null + new "test params: -f $cfg -y $fyang" if [ $BE -ne 0 ]; then new "kill old backend" @@ -184,4 +211,5 @@ fi # kill backend stop_backend -f $cfg + rm -rf $dir