[ovs-dev] [PATCH v2] ovn-northd: Revert "ovn-northd: Only run idl loop if something changed"

[Russell Bryant]

On 02/04/2016 09:41 AM, Numan Siddique wrote:
> This reverts f20396e051ea4fd623bfc022cc78d9bd52a850e5
> 
> f20396e.. was required to fix the failing ovn-sbctl testsuites.
> commit fb496f92ca1eeead8760b5cdfd857165f64deadf addresses these failing
> test cases in a proper way.
> 
> With commit f20396e.. its possible that commit to the southbound db is
> delayed. When ovnnb_db_run is called, and if ctx->ovnsb_txn is NULL,
> ovnnb_db_run returns immediately without generating the logical flows.
> ovnnb_db_run is not called again until the northbound db seqno changes.
> 
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> ---
>  ovn/northd/ovn-northd.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 8083e25..e6271cf 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1877,7 +1877,6 @@ add_column_noalert(struct ovsdb_idl *idl,
>  int
>  main(int argc, char *argv[])
>  {
> -    unsigned int ovnnb_seqno, ovnsb_seqno;
>      int res = EXIT_SUCCESS;
>      struct unixctl_server *unixctl;
>      int retval;
> @@ -1945,9 +1944,6 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
>  
> -    ovnnb_seqno = ovsdb_idl_get_seqno(ovnnb_idl_loop.idl);
> -    ovnsb_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
> -
>      /* Main loop. */
>      exiting = false;
>      while (!exiting) {
> @@ -1958,14 +1954,8 @@ main(int argc, char *argv[])
>              .ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
>          };
>  
> -        if (ovnnb_seqno != ovsdb_idl_get_seqno(ctx.ovnnb_idl)) {
> -            ovnnb_seqno = ovsdb_idl_get_seqno(ctx.ovnnb_idl);
> -            ovnnb_db_run(&ctx);
> -        }
> -        if (ovnsb_seqno != ovsdb_idl_get_seqno(ctx.ovnsb_idl)) {
> -            ovnsb_seqno = ovsdb_idl_get_seqno(ctx.ovnsb_idl);
> -            ovnsb_db_run(&ctx);
> -        }
> +        ovnnb_db_run(&ctx);
> +        ovnsb_db_run(&ctx);
>  
>          unixctl_server_run(unixctl);
>          unixctl_server_wait(unixctl);
> 


Thanks.  On the surface this looks like a fine optimization, but I see
that it breaks things in some cases.  I applied this patch to master.

-- 
Russell Bryant