| Message ID | 1455986150-26728-2-git-send-email-lirans@il.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, Feb 20, 2016 at 8:35 AM, Liran Schour <lirans@il.ibm.com> wrote: > In case that we flushed everything already, we can immeidately return NULL. > > Signed-off-by: Liran Schour <lirans@il.ibm.com> > --- > ovsdb/monitor.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > index 5ae9cdb..1a07f19 100644 > --- a/ovsdb/monitor.c > +++ b/ovsdb/monitor.c > @@ -733,6 +733,10 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon, > uint64_t prev_txn = *unflushed; > uint64_t next_txn = dbmon->n_transactions + 1; > > + if (prev_txn == next_txn) { > + return NULL; > + } > + > Thanks for reporting the issue. The change as is breaks the unit tests. But the optimziation does make sense. After looking further, I was able to find a bug. I have posted a patch series that fixes this bug and folded the suggested optimization (in a slightly different manner). . Would you please review them: http://openvswitch.org/pipermail/dev/2016-February/066513.html Thanks! > /* Return a clone of cached json if one exists. Otherwise, > * generate a new one and add it to the cache. */ > cache_node = ovsdb_monitor_json_cache_search(dbmon, version, > prev_txn); > -- > 2.1.4 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
On Mon, Feb 22, 2016 at 01:13:07AM -0800, Andy Zhou wrote: > On Sat, Feb 20, 2016 at 8:35 AM, Liran Schour <lirans@il.ibm.com> wrote: > > > In case that we flushed everything already, we can immeidately return NULL. > > > > Signed-off-by: Liran Schour <lirans@il.ibm.com> > > --- > > ovsdb/monitor.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > > index 5ae9cdb..1a07f19 100644 > > --- a/ovsdb/monitor.c > > +++ b/ovsdb/monitor.c > > @@ -733,6 +733,10 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon, > > uint64_t prev_txn = *unflushed; > > uint64_t next_txn = dbmon->n_transactions + 1; > > > > + if (prev_txn == next_txn) { > > + return NULL; > > + } > > + > > > Thanks for reporting the issue. The change as is breaks the unit tests. But > the optimziation > does make sense. After looking further, I was able to find a bug. > > I have posted a patch series that fixes this bug and folded the suggested > optimization > (in a slightly different manner). . Would you please review them: > > http://openvswitch.org/pipermail/dev/2016-February/066513.html I don't think that's the right message reference. Do you mean http://openvswitch.org/pipermail/dev/2016-February/066515.html ?
On Mon, Feb 22, 2016 at 9:28 AM, Ben Pfaff <blp@ovn.org> wrote: > On Mon, Feb 22, 2016 at 01:13:07AM -0800, Andy Zhou wrote: > > On Sat, Feb 20, 2016 at 8:35 AM, Liran Schour <lirans@il.ibm.com> wrote: > > > > > In case that we flushed everything already, we can immeidately return > NULL. > > > > > > Signed-off-by: Liran Schour <lirans@il.ibm.com> > > > --- > > > ovsdb/monitor.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > > > index 5ae9cdb..1a07f19 100644 > > > --- a/ovsdb/monitor.c > > > +++ b/ovsdb/monitor.c > > > @@ -733,6 +733,10 @@ ovsdb_monitor_get_update(struct ovsdb_monitor > *dbmon, > > > uint64_t prev_txn = *unflushed; > > > uint64_t next_txn = dbmon->n_transactions + 1; > > > > > > + if (prev_txn == next_txn) { > > > + return NULL; > > > + } > > > + > > > > > Thanks for reporting the issue. The change as is breaks the unit tests. > But > > the optimziation > > does make sense. After looking further, I was able to find a bug. > > > > I have posted a patch series that fixes this bug and folded the suggested > > optimization > > (in a slightly different manner). . Would you please review them: > > > > http://openvswitch.org/pipermail/dev/2016-February/066513.html > > I don't think that's the right message reference. Do you mean > http://openvswitch.org/pipermail/dev/2016-February/066515.html > ? > Yes. Thanks for the correction.
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index 5ae9cdb..1a07f19 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -733,6 +733,10 @@ ovsdb_monitor_get_update(struct ovsdb_monitor *dbmon, uint64_t prev_txn = *unflushed; uint64_t next_txn = dbmon->n_transactions + 1; + if (prev_txn == next_txn) { + return NULL; + } + /* Return a clone of cached json if one exists. Otherwise, * generate a new one and add it to the cache. */ cache_node = ovsdb_monitor_json_cache_search(dbmon, version, prev_txn);
In case that we flushed everything already, we can immeidately return NULL. Signed-off-by: Liran Schour <lirans@il.ibm.com> --- ovsdb/monitor.c | 4 ++++ 1 file changed, 4 insertions(+)