| Message ID | 20190709101630.9691-1-dalvarez@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovsdb-server: drop all connections on read/write status change | expand |
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>
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.
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?
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?
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
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(-)