Optimized get config xpath of large lists, such as a[x=1000] in a list of 100000s a:s.

CHANGELOG.md

@@ -81,6 +81,7 @@ Expected: July 2020
   
 ### Minor changes
 
+* Optimized get config xpath of large lists, such as `a[x=1000]` in a list of 100000s `a:s`.
 * Added docker support for three restconf modes: nginx/fcgi(default); evhtp ; and none.
 * Added [Vagrant tests](test/vagrant/README.md)
 * Added new function `clicon_xml2str()` to complement xml_print and others that returns a malloced string.

lib/src/clixon_datastore_read.c

@@ -120,6 +120,115 @@ singleconfigroot(cxobj  *xt,
  done:
     return retval;
 }
+
+/*! Recurse up from x0 up to x0t then create objects from x1t down to new object x1
+ */
+static int
+xml_copy_bottom_recurse(cxobj  *x0t, 
+			cxobj  *x0,
+			cxobj  *x1t,
+			cxobj **x1pp)
+{
+    int        retval = -1;
+    cxobj     *x0p = NULL;
+    cxobj     *x1p = NULL;
+    cxobj     *x1 = NULL;
+    cxobj     *x1a = NULL;
+    cxobj     *x0a = NULL;
+    cxobj     *x0k;
+    cxobj     *x1k;
+    yang_stmt *y = NULL;
+    cvec      *cvk = NULL; /* vector of index keys */
+    cg_var    *cvi;
+    char      *keyname;
+
+    if (x0 == x0t){
+	*x1pp = x1t;
+	goto ok;
+    }
+    if ((x0p = xml_parent(x0)) == NULL){
+	clicon_err(OE_XML, EFAULT, "Reached top of tree");
+	goto done;
+    }
+    if (xml_copy_bottom_recurse(x0t, x0p, x1t, &x1p) < 0)
+	goto done;
+    y = xml_spec(x0);
+    /* Look if it exists */
+    if (match_base_child(x1p, x0, y, &x1) < 0)
+	goto done;
+    if (x1 == NULL){ /* If not, create it and copy it one level only */
+	if ((x1 = xml_new(xml_name(x0), x1p, CX_ELMNT)) == NULL)
+	    goto done;
+	if (xml_copy_one(x0, x1) < 0)
+	    goto done;
+	/* Copy all attributes */
+	x0a = NULL;
+	while ((x0a = xml_child_each(x0, x0a, -1)) != NULL) {
+	    /* Assume ordered, skip after attributes */
+	    if (xml_type(x0a) != CX_ATTR)
+		break;
+	    if ((x1a = xml_new(xml_name(x0a), x1, CX_ATTR)) == NULL)
+		goto done;
+	    if (xml_copy_one(x0a, x1a) < 0)
+		goto done;
+	}
+	/* Key nodes in lists are copied */
+	if (y && yang_keyword_get(y) == Y_LIST){
+	    /* Loop over all key variables */
+	    cvk = yang_cvec_get(y); /* Use Y_LIST cache, see ys_populate_list() */
+	    cvi = NULL;
+	    /* Iterate over individual keys  */
+	    while ((cvi = cvec_each(cvk, cvi)) != NULL) {
+		keyname = cv_string_get(cvi);	    
+		if ((x0k = xml_find_type(x0, NULL, keyname, CX_ELMNT)) != NULL){
+		    if ((x1k = xml_new(keyname, x1, CX_ELMNT)) == NULL)
+			goto done;
+		    if (xml_copy(x0k, x1k) < 0) 
+			goto done;
+		}
+
+	    }
+	}
+    }
+    *x1pp = x1;
+ ok:
+    retval = 0;
+ done:
+    return retval;
+}
+
+static int
+xml_copy_from_bottom(cxobj  *x0t, 
+		     cxobj  *x0,
+		     cxobj  *x1t)
+{
+    int        retval = -1;
+    cxobj     *x1p    = NULL;
+    cxobj     *x0p    = NULL;
+    cxobj     *x1     = NULL;
+    yang_stmt *y      = NULL;
+    
+    if (x0 == x0t)
+	goto ok;
+    x0p = xml_parent(x0);
+    if (xml_copy_bottom_recurse(x0t, x0p, x1t, &x1p) < 0)
+	return -1;
+    y = xml_spec(x0);
+    /* Look if it exists */
+    if (match_base_child(x1p, x0, y, &x1) < 0)
+	goto done;
+    if (x1 == NULL){ /* If not, create it and copy complete tree */
+	if ((x1 = xml_new(xml_name(x0), x1p, CX_ELMNT)) == NULL)
+	    goto done;
+	if (xml_copy(x0, x1) < 0)
+	    goto done;
+    }
+ ok:
+    retval = 0;
+ done:
+    return retval;
+}
+
 /*! Given XML tree x0 with marked nodes, copy marked nodes to new tree x1
  * Two marks are used: XML_FLAG_MARK and XML_FLAG_CHANGE
  *
@@ -563,20 +672,33 @@ xmldb_get_cache(clicon_handle    h,
 	goto done;
     xml_spec_set(x1t, xml_spec(x0t));
 
-    /* Iterate through the match vector
+    if (xlen < 1000){
-     * For every node found in x0, mark the tree up to t1
+	/* This is optimized for the case when the tree is large and xlen is small
-     */
+	 * If the tree is large and xlen too, then the other is better.
-    for (i=0; i<xlen; i++){
+	 */
-	x0 = xvec[i];
+	for (i=0; i<xlen; i++){
-	xml_flag_set(x0, XML_FLAG_MARK);
+	    x0 = xvec[i];
-	xml_apply_ancestor(x0, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
+	    if (xml_copy_from_bottom(x0t, x0, x1t) < 0) /* config */
+		goto done;
+	}
+    }
+    else {
+	/* Iterate through the match vector
+	 * For every node found in x0, mark the tree up to t1
+	 * XXX can we do this directly from xvec?
+	 */
+	for (i=0; i<xlen; i++){
+	    x0 = xvec[i];
+	    xml_flag_set(x0, XML_FLAG_MARK);
+	    xml_apply_ancestor(x0, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
+	}
+	if (xml_copy_marked(x0t, x1t) < 0) /* config */
+	    goto done;
+	if (xml_apply(x0t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
+	    goto done;
+	if (xml_apply(x1t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
+	    goto done;
     }
-    if (xml_copy_marked(x0t, x1t) < 0) /* config */
-	goto done;
-    if (xml_apply(x0t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
-	goto done;
-    if (xml_apply(x1t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
-	goto done;
     /* x1t is wrong here should be <config><system>.. but is <system>.. */
     /* XXX where should we apply default values once? */
     if (xml_default_recurse(x1t) < 0)

lib/src/clixon_xml_sort.c

@@ -1129,11 +1129,11 @@ xml_sort_verify(cxobj *x0,
 /*! Given child tree x1c, find (first) matching child in base tree x0 and return as x0cp
  * @param[in]  x0      Base tree node
  * @param[in]  x1c     Modification tree child
- * @param[in]  yc      Yang spec of tree child
+ * @param[in]  yc      Yang spec of tree child. If null revert to linear search.
  * @param[out] x0cp    Matching base tree child (if any)
  * @retval     0       OK
  * @retval    -1       Error
- * XXX: only handles first match
+ * @note only handles first match
  */
 int
 match_base_child(cxobj      *x0, 
@@ -1153,6 +1153,11 @@ match_base_child(cxobj      *x0,
     clixon_xvec *xvec = NULL;
     
     *x0cp = NULL; /* init return value */
+    /* Revert to simple xml lookup if no yang */
+    if (yc == NULL){ 
+	*x0cp = xml_find(x0, xml_name(x1c));
+	goto ok;
+    }
     /* Special case is if yc parent (yp) is choice/case
      * then find x0 child with same yc even though it does not match lexically
      * However this will give another y0c != yc

test/test_perf_cli.sh

@@ -100,15 +100,13 @@ new "netconf write large config"
 expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Here, there are $perfnr entries in candidate
-new "netconf write large config again"
-expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Now commit it from candidate to running 
 new "netconf commit large config"
 expecteof "time -p $clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
-# CLI get (XXX why does this take so much time?)
+# CLI get. This takes a lot of time because expand_dbvar gets perfnr (eg 100000) entries
-# See: EXPAND_ONLY_INTERACTIVE in cligen. If set it is acceptable but there are some side-effects
+# and loops over it.
 new "cli get $perfreq small config 1 key index"
 { time -p for (( i=0; i<$perfreq; i++ )); do
     rnd=$(( ( RANDOM % $perfnr ) ))

test/test_perf_netconf.sh

@@ -101,8 +101,6 @@ new "netconf write large config"
 expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Here, there are $perfnr entries in candidate
-new "netconf write large config again"
-expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Now commit it from candidate to running 
 new "netconf commit large config"
@@ -110,19 +108,26 @@ expecteof "time -p $clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<r
 
 # Now commit it again from candidate (validation takes time when
 # comparing to existing)
-new "netconf commit large config again"
-expecteof "time -p $clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Having a large db, get and put single entries many times
 # Note same entries in the range alreay there, db has same size
 
 # NETCONF get 1 key index
+# Note this is done by streaming input into one single netconf client. it is much
+# slower if it is started and stopped for each request. (next)
 new "netconf get $perfreq small config 1 key index"
 { time -p for (( i=0; i<$perfreq; i++ )); do
     rnd=$(( ( RANDOM % $perfnr ) ))
     echo "<rpc><get-config><source><candidate/></source><filter type=\"xpath\" select=\"/ex:x/ex:y[ex:a=$rnd]\" xmlns:ex=\"urn:example:clixon\"/></get-config></rpc>]]>]]>"
 done | $clixon_netconf -qf $cfg > /dev/null; } 2>&1 | awk '/real/ {print $2}'
 
+new "netconf get $perfreq small config 1 key index start/stop"
+{ time -p for (( i=0; i<$perfreq; i++ )); do
+    rnd=$(( ( RANDOM % $perfnr ) ))
+    echo "<rpc><get-config><source><candidate/></source><filter type=\"xpath\" select=\"/ex:x/ex:y[ex:a=$rnd]\" xmlns:ex=\"urn:example:clixon\"/></get-config></rpc>]]>]]>"  | $clixon_netconf -qf $cfg > /dev/null; 
+done
+}  2>&1 | awk '/real/ {print $2}'
+
 # NETCONF get 1 key and one non-key index
 new "netconf get $perfreq small config 1 key + 1 non-key index"
 { time -p for (( i=0; i<$perfreq; i++ )); do

test/test_perf_restconf.sh

@@ -112,8 +112,6 @@ new "netconf write large config"
 expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Here, there are $perfnr entries in candidate
-new "netconf write large config again"
-expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
 
 # Now commit it from candidate to running 
 new "netconf commit large config"

test/test_rpc.sh

@@ -142,24 +142,24 @@ new "netconf edit-config ok"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target><config/></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
 
 if ! $YANG_UNKNOWN_ANYDATA ; then
-new "netconf edit-config extra arg should fail"
+new "netconf edit-config extra arg: should fail"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target><extra/><config/></edit-config></rpc>]]>]]>' "^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>extra</bad-element></error-info><error-severity>error</error-severity><error-message>Failed to find YANG spec of XML node: extra with parent: edit-config in namespace: urn:ietf:params:xml:ns:netconf:base:1.0</error-message></rpc-error></rpc-reply>]]>]]>$"
 fi
 
-new "netconf edit-config empty target should fail"
+new "netconf edit-config empty target: should fail"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target/><config/></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>data-missing</error-tag><error-app-tag>missing-choice</error-app-tag><error-info><missing-choice>config-target</missing-choice></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
 
-new "netconf edit-config missing target should fail"
+new "netconf edit-config missing target: should fail"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><config/></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>target</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory variable</error-message></rpc-error></rpc-reply>]]>]]>$'
 
-new "netconf edit-config missing config should fail"
+new "netconf edit-config missing config: should fail"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>data-missing</error-tag><error-app-tag>missing-choice</error-app-tag><error-info><missing-choice>edit-content</missing-choice></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
 
 # Negative errors (namespace/module missing)
-new "netconf wrong rpc namespace (should fail)"
+new "netconf wrong rpc namespace: should fail"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc xmlns="urn:example:xxx"><get/></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>get</bad-element></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
 
-new "restconf wrong rpc (should fail"
+new "restconf wrong rpc: should fail"
 expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" u $RCPROTO://localhost/restconf/operations/clixon-foo:get)" 0 'HTTP/1.1 412 Precondition Failed' '{"ietf-restconf:errors":{"error":{"error-type":"protocol","error-tag":"operation-failed","error-severity":"error","error-message":"yang module not found"}}}'
 
 if [ $RC -ne 0 ]; then

test/test_upgrade_module.sh

@@ -299,16 +299,16 @@ EOF
 # There is some issue with having different payloads in the config file
 # That is why there are tests with different payloads
 
-new "b payload only---------"
+new "b payload only"
 testall '<dummy xmlns="urn:example:b"/>' '<dummy xmlns="urn:example:b"/>'
 
 new "b payload and interfaces payload---------"
 testall '<dummy xmlns="urn:example:b"/><dummy xmlns="urn:example:interfaces"/>' '<dummy xmlns="urn:example:b"/>'
 
-new "a payload only---------"
+new "a payload only"
 testall '<dummy xmlns="urn:example:interfaces"/>' ''
 
-new "empty payload---------"
+new "empty payload"
 testall '' ''
 
 rm -rf $dir