On Tue, Aug 02, 2016 at 04:25:03PM -0500, Ryan Moats wrote:
> First, this code from ./ovsdb/execution.c (L693-707) looks
> a bit funny:
> 
> if (!strcmp(json_string(until), "==") != equal) {
>     if (timeout && x->elapsed_msec >= timeout_msec) {
>         if (x->elapsed_msec) {
>             error = ovsdb_error("timed out",
>                                 "\"wait\" timed out after %lld ms",
>                                 x->elapsed_msec);
>         } else {
>             error = ovsdb_error("timed out", "\"wait\" timed out");
>         }
>     } else {
>         /* ovsdb_execute() will change this, if triggers really are
>          * supported. */
>         error = ovsdb_error("not supported", "triggers not supported");
>     }
> }
> 
> Specifically, returning a message for "timed out" when a
> timeout of 0 seconds has been specified and the conditional
> has not matched is misleading at best. I think it would make
> more sense to say "wait condition not met" or something like
> that.

The second string in the ovsdb_error() call can be whatever we want, and
if you think that something else is a better explanation then I'm happy
to take it, but RFC 7047 requires the string "timed out" for the error
type (see section 5.2.6 "Wait").

> The second issue is a bit more serious.  Right now, before the
> networking-ovn IDL will add a new logical switch port and
> associated ACLs (and if my memory serves me, it creates seven
> for each port), there are a pair of wait clauses that have to
> be met.  The first of these is that the ports and ACLs for
> the Logical Switch have to have a particular list of UUIDs
> in each, and each entire list is sent in the transaction.
> 
> Is this strictly necessary?  I ask because I'm looking at
> scaling to a point where the wait condition will have
> 8000 uuids for the logical switch ports part and
> 56000 uuids for the ACLs part and I haven't yet figured
> out why it is needed...

Presumably this means that networking-ovn is calling "verify" on the
column in question.  Probably, networking-ovn should use the partial map
update functionality introduced in commit f199df26e8e28 "ovsdb-idl: Add
partial map updates functionality."  I don't know whether it's in the
Python IDL yet.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to