[ovs-dev] [PATCH monitor_cond V5 04/18] ovsdb: generate update notifications for monitor_cond session
Liran Schour
LIRANS at il.ibm.com
Tue Mar 22 21:24:14 UTC 2016
Ben Pfaff <blp at ovn.org> wrote on 22/03/2016 07:23:33 PM:
> On Fri, Mar 04, 2016 at 08:08:59AM +0000, Liran Schour wrote:
> > Hold session's conditions in ovsdb_monitor_session_condition. Pass it
> > to ovsdb_monitor for generating "update2" notifications.
> > Add functions that can generate "update2" notification for a
> > "monitor_cond" session.
> > JSON cache is enabled only for session's with true condition only.
> > "monitor_cond" and "monitor_cond_change" are RFC 7047 extensions
> > described by ovsdb-server(1) manpage.
> >
> > Signed-off-by: Liran Schour <lirans at il.ibm.com>
>
> I think that ovsdb_monitor_table_condition_set() has a memory leak in
> the error case; it does not free the 'error' returned to it.
>
Right. Will fix that.
> I'm a little confused about ovsdb_monitor_session_condition. Why is
> this a separate data structure? That is, why is there a separate shash
> of conditions rather than incorporating a condition into
> ovsdb_monitor_table?
>
A single ovsdb_monitor instance can be used by many jsonrpc sessions (as
long as they share the same set of tables and columns).
Ovsdb_monitor_condition_session is holding the condition state per each
jsonrpc session. Each session, in ovsdb_monitor, can have a different set
of conditions and it is being represented by
ovsdb_monitor_session_condition with ovsdb_monitor_table_condition per
table.
The trivial option was to divide ovsdb_monitor into 2 different instances
if we have 2 sessions with different sets of conditions. But, condition is
changing during the lifetime of monitor session. And by holding sessions
with different conditions under the same ovsdb_monitor we can reduce the
computation needed to insert row changes to ovsdb_monitor. (insert once
instead of twice)
> If a separate data structure is really needed then possibly it would be
> better to only include the non-"true" conditions in the shash. Then it
> would be easy to determine how many non-"true" conditions there were
> (shash_count(conditions)) and hence whether there were any at all.
>
Something like this can be done however I do not support this. Conditions
might change and it is needed to keep the condition state even if it is
true (implicit or explicit).
> I'll continue reviewing the series once I understand this point.
Hope that it is clearer now. Thanks.
More information about the dev
mailing list