diff mbox

[ovs-dev,v9,02/10] Present tracked changes in increasing change number order

Message ID 1457730385-28923-3-git-send-email-rmoats@us.ibm.com
State Accepted
Headers show

Commit Message

Ryan Moats March 11, 2016, 9:06 p.m. UTC
From: RYAN D. MOATS <rmoats@us.ibm.com>

Currently changes are added to the front of the track list, so
they are looped through in LIFO order. Incremental processing
is more efficient with a FIFO presentation, so
(1) add new changes to the back of the track list, and
(2) move updated changes to the back of the track list

Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com>
---
 lib/ovsdb-idl.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

Comments

Ben Pfaff March 22, 2016, 9:37 p.m. UTC | #1
On Fri, Mar 11, 2016 at 03:06:17PM -0600, Ryan Moats wrote:
> From: RYAN D. MOATS <rmoats@us.ibm.com>
> 
> Currently changes are added to the front of the track list, so
> they are looped through in LIFO order. Incremental processing
> is more efficient with a FIFO presentation, so
> (1) add new changes to the back of the track list, and
> (2) move updated changes to the back of the track list
> 
> Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com>

I applied this because it makes a lot of sense to me: it causes changes
to be presented in their natural order, that is, in the order in which
they were received.  I'm copying Shad, the original author of the column
tracking code, on this message in case he wants to advocate that the
previous order was superior.

Thanks,

Ben.
Ben Pfaff March 22, 2016, 10:14 p.m. UTC | #2
On Tue, Mar 22, 2016 at 02:37:50PM -0700, Ben Pfaff wrote:
> On Fri, Mar 11, 2016 at 03:06:17PM -0600, Ryan Moats wrote:
> > From: RYAN D. MOATS <rmoats@us.ibm.com>
> > 
> > Currently changes are added to the front of the track list, so
> > they are looped through in LIFO order. Incremental processing
> > is more efficient with a FIFO presentation, so
> > (1) add new changes to the back of the track list, and
> > (2) move updated changes to the back of the track list
> > 
> > Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com>
> 
> I applied this because it makes a lot of sense to me: it causes changes
> to be presented in their natural order, that is, in the order in which
> they were received.  I'm copying Shad, the original author of the column
> tracking code, on this message in case he wants to advocate that the
> previous order was superior.

Oops, trying again with Shad's email address fixed.
Ryan Moats March 23, 2016, 1:26 p.m. UTC | #3
Ben Pfaff <blp@ovn.org> wrote on 03/22/2016 04:37:50 PM:

> From: Ben Pfaff <blp@ovn.org>
> To: Ryan Moats/Omaha/IBM@IBMUS, Shad Ansari <shad.ansar@hpe.com>
> Cc: dev@openvswitch.org
> Date: 03/22/2016 04:38 PM
> Subject: Re: [ovs-dev, v9, 02/10] Present tracked changes in
> increasing change number order
>
> On Fri, Mar 11, 2016 at 03:06:17PM -0600, Ryan Moats wrote:
> > From: RYAN D. MOATS <rmoats@us.ibm.com>
> >
> > Currently changes are added to the front of the track list, so
> > they are looped through in LIFO order. Incremental processing
> > is more efficient with a FIFO presentation, so
> > (1) add new changes to the back of the track list, and
> > (2) move updated changes to the back of the track list
> >
> > Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com>
>
> I applied this because it makes a lot of sense to me: it causes changes
> to be presented in their natural order, that is, in the order in which
> they were received.  I'm copying Shad, the original author of the column
> tracking code, on this message in case he wants to advocate that the
> previous order was superior.
>
> Thanks,
>
> Ben.
>

Thanks Ben, and FYI, I've got lots of reasons (read scars) for why I think
the previous order is *not* superior - I'll be happy to share them (if need
be)...

Ryan (regXboi)
Ansari, Shad March 23, 2016, 4:14 p.m. UTC | #4
> -----Original Message-----

> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Ben Pfaff

> Sent: Tuesday, March 22, 2016 2:38 PM

> To: Ryan Moats <rmoats@us.ibm.com>; Shad Ansari <shad.ansar@hpe.com>

> Cc: dev@openvswitch.org

> Subject: Re: [ovs-dev] [ovs-dev, v9, 02/10] Present tracked changes in increasing

> change number order

> 

> On Fri, Mar 11, 2016 at 03:06:17PM -0600, Ryan Moats wrote:

> > From: RYAN D. MOATS <rmoats@us.ibm.com>

> >

> > Currently changes are added to the front of the track list, so

> > they are looped through in LIFO order. Incremental processing

> > is more efficient with a FIFO presentation, so

> > (1) add new changes to the back of the track list, and

> > (2) move updated changes to the back of the track list

> >

> > Signed-off-by: RYAN D. MOATS <rmoats@us.ibm.com>

> 

> I applied this because it makes a lot of sense to me: it causes changes

> to be presented in their natural order, that is, in the order in which

> they were received.  I'm copying Shad, the original author of the column

> tracking code, on this message in case he wants to advocate that the

> previous order was superior.

> 

> Thanks,

> 


Makes sense. Thanks.

Acked-by: Shad Ansari <shad.ansari@hpe.com>
diff mbox

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 4cb1c81..5dc8565 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1350,10 +1350,11 @@  ovsdb_idl_row_change__(struct ovsdb_idl_row *row, const struct json *row_json,
                         = row->table->change_seqno[change]
                         = row->table->idl->change_seqno + 1;
                     if (table->modes[column_idx] & OVSDB_IDL_TRACK) {
-                        if (list_is_empty(&row->track_node)) {
-                            list_push_front(&row->table->track_list,
-                                            &row->track_node);
+                        if (!list_is_empty(&row->track_node)) {
+                            list_remove(&row->track_node);
                         }
+                        list_push_back(&row->table->track_list,
+                                       &row->track_node);
                         if (!row->updated) {
                             row->updated = bitmap_allocate(class->n_columns);
                         }
@@ -1572,7 +1573,7 @@  ovsdb_idl_row_destroy(struct ovsdb_idl_row *row)
                 = row->table->idl->change_seqno + 1;
         }
         if (list_is_empty(&row->track_node)) {
-            list_push_front(&row->table->track_list, &row->track_node);
+            list_push_back(&row->table->track_list, &row->track_node);
         }
     }
 }