[ovs-dev] ovsdb-server: drop all connections on read/write status change
diff mbox series

Message ID 20190709101630.9691-1-dalvarez@redhat.com
State New
Headers show
Series
  • [ovs-dev] ovsdb-server: drop all connections on read/write status change
Related show

Commit Message

Daniel Alvarez July 9, 2019, 10:16 a.m. UTC
Prior to this patch, only db change aware connections were dropped
on a read/write status change. However, current schema in OVN does
not allow clients to monitor whether a particular DB changes this
status. In order to accomplish this, we'd need to change the schema
and adapting ovsdb-server and existing clients.

Before tackling that, this patch is changing ovsdb-server to drop
*all* the existing connections upon a read/write status change. This
will force clients to reconnect and honor the change.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-July/048981.html
Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
---
 ovsdb/jsonrpc-server.c |  2 +-
 tests/ovn.at           | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

Comments

Lucas Alvares Gomes July 9, 2019, 10:58 a.m. UTC | #1
Glad to see this being fixed, thanks Daniel.

On Tue, Jul 9, 2019 at 11:22 AM Daniel Alvarez <dalvarez@redhat.com> wrote:
>
> Prior to this patch, only db change aware connections were dropped
> on a read/write status change. However, current schema in OVN does
> not allow clients to monitor whether a particular DB changes this
> status. In order to accomplish this, we'd need to change the schema
> and adapting ovsdb-server and existing clients.
>
> Before tackling that, this patch is changing ovsdb-server to drop
> *all* the existing connections upon a read/write status change. This
> will force clients to reconnect and honor the change.
>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-July/048981.html
> Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
> ---
>  ovsdb/jsonrpc-server.c |  2 +-
>  tests/ovn.at           | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 4dda63a9d..ddbbc2e94 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -365,7 +365,7 @@ ovsdb_jsonrpc_server_set_read_only(struct ovsdb_jsonrpc_server *svr,
>  {
>      if (svr->read_only != read_only) {
>          svr->read_only = read_only;
> -        ovsdb_jsonrpc_server_reconnect(svr, false,
> +        ovsdb_jsonrpc_server_reconnect(svr, true,
>                                         xstrdup(read_only
>                                                 ? "making server read-only"
>                                                 : "making server read/write"));
> diff --git a/tests/ovn.at b/tests/ovn.at
> index daace1128..4da7059b3 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -14313,3 +14313,24 @@ OVN_CHECK_PACKETS([hv2/vif22-tx.pcap], [vif22.expected])
>  OVN_CLEANUP([hv1],[hv2])
>
>  AT_CLEANUP
> +
> +# Run ovn-nbctl in daemon mode, change to a backup database and verify that
> +# an insert operation is not allowed.
> +AT_SETUP([ovn -- can't write to a backup database server instance])
> +ovn_start
> +on_exit 'kill $(cat ovn-nbctl.pid)'
> +export OVN_NB_DAEMON=$(ovn-nbctl --pidfile --detach)
> +
> +AT_CHECK([ovn-nbctl ls-add sw0])
> +as ovn-nb
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/sync-status | grep active | wc -l], [0], [1
> +])
> +ovs-appctl -t ovsdb-server ovsdb-server/set-active-ovsdb-server tcp:192.0.2.2:6641
> +ovs-appctl -t ovsdb-server ovsdb-server/connect-active-ovsdb-server
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/sync-status | grep -c backup], [0], [1
> +])
> +AT_CHECK([ovn-nbctl ls-add sw1], [1], [ignore],
> +[ovn-nbctl: transaction error: {"details":"insert operation not allowed when database server is in read only mode","error":"not allowed"}
> +])
> +
> +AT_CLEANUP
> --
> 2.21.0 (Apple Git-120)
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Acked-By: Lucas Alvares Gomes <lucasagomes@gmail.com>
Ben Pfaff July 10, 2019, 8:24 p.m. UTC | #2
On Tue, Jul 09, 2019 at 12:16:30PM +0200, Daniel Alvarez wrote:
> Prior to this patch, only db change aware connections were dropped
> on a read/write status change. However, current schema in OVN does
> not allow clients to monitor whether a particular DB changes this
> status. In order to accomplish this, we'd need to change the schema
> and adapting ovsdb-server and existing clients.
> 
> Before tackling that, this patch is changing ovsdb-server to drop
> *all* the existing connections upon a read/write status change. This
> will force clients to reconnect and honor the change.
> 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-July/048981.html
> Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>

Thanks, applied to master.
Daniel Alvarez July 12, 2019, 1:06 p.m. UTC | #3
On Wed, Jul 10, 2019 at 10:24 PM Ben Pfaff <blp@ovn.org> wrote:
>
> On Tue, Jul 09, 2019 at 12:16:30PM +0200, Daniel Alvarez wrote:
> > Prior to this patch, only db change aware connections were dropped
> > on a read/write status change. However, current schema in OVN does
> > not allow clients to monitor whether a particular DB changes this
> > status. In order to accomplish this, we'd need to change the schema
> > and adapting ovsdb-server and existing clients.
> >
> > Before tackling that, this patch is changing ovsdb-server to drop
> > *all* the existing connections upon a read/write status change. This
> > will force clients to reconnect and honor the change.
> >
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-July/048981.html
> > Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
>
> Thanks, applied to master.
Thanks a lot Ben. Do you think it would make sense to backport it as
far as we could get to the stable branches?
Ben Pfaff July 12, 2019, 3:21 p.m. UTC | #4
On Fri, Jul 12, 2019 at 03:06:34PM +0200, Daniel Alvarez Sanchez wrote:
> On Wed, Jul 10, 2019 at 10:24 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Tue, Jul 09, 2019 at 12:16:30PM +0200, Daniel Alvarez wrote:
> > > Prior to this patch, only db change aware connections were dropped
> > > on a read/write status change. However, current schema in OVN does
> > > not allow clients to monitor whether a particular DB changes this
> > > status. In order to accomplish this, we'd need to change the schema
> > > and adapting ovsdb-server and existing clients.
> > >
> > > Before tackling that, this patch is changing ovsdb-server to drop
> > > *all* the existing connections upon a read/write status change. This
> > > will force clients to reconnect and honor the change.
> > >
> > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-July/048981.html
> > > Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
> >
> > Thanks, applied to master.
> Thanks a lot Ben. Do you think it would make sense to backport it as
> far as we could get to the stable branches?

It has patch conflicts for the backport (in the test), would you mind
sending a backported version?

Patch
diff mbox series

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 4dda63a9d..ddbbc2e94 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -365,7 +365,7 @@  ovsdb_jsonrpc_server_set_read_only(struct ovsdb_jsonrpc_server *svr,
 {
     if (svr->read_only != read_only) {
         svr->read_only = read_only;
-        ovsdb_jsonrpc_server_reconnect(svr, false,
+        ovsdb_jsonrpc_server_reconnect(svr, true,
                                        xstrdup(read_only
                                                ? "making server read-only"
                                                : "making server read/write"));
diff --git a/tests/ovn.at b/tests/ovn.at
index daace1128..4da7059b3 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14313,3 +14313,24 @@  OVN_CHECK_PACKETS([hv2/vif22-tx.pcap], [vif22.expected])
 OVN_CLEANUP([hv1],[hv2])
 
 AT_CLEANUP
+
+# Run ovn-nbctl in daemon mode, change to a backup database and verify that
+# an insert operation is not allowed.
+AT_SETUP([ovn -- can't write to a backup database server instance])
+ovn_start
+on_exit 'kill $(cat ovn-nbctl.pid)'
+export OVN_NB_DAEMON=$(ovn-nbctl --pidfile --detach)
+
+AT_CHECK([ovn-nbctl ls-add sw0])
+as ovn-nb
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/sync-status | grep active | wc -l], [0], [1
+])
+ovs-appctl -t ovsdb-server ovsdb-server/set-active-ovsdb-server tcp:192.0.2.2:6641
+ovs-appctl -t ovsdb-server ovsdb-server/connect-active-ovsdb-server
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/sync-status | grep -c backup], [0], [1
+])
+AT_CHECK([ovn-nbctl ls-add sw1], [1], [ignore],
+[ovn-nbctl: transaction error: {"details":"insert operation not allowed when database server is in read only mode","error":"not allowed"}
+])
+
+AT_CLEANUP