Message ID | 20191014091939.449-1-nusiddiq@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,v2] ovsdb-server: Don't drop all connections on read/write status change. | expand |
On Mon, Oct 14, 2019 at 11:33 AM <nusiddiq@redhat.com> wrote: > > From: Numan Siddique <nusiddiq@redhat.com> > > The commit [1] force drops all connections when the db read/write status changes. > Prior to the commit [1], when there was read/write status change, the existing > jsonrpc sessions with 'db_change_aware' set to true, were not updated with the > changed 'read_only' value. If the db status was changed to 'standby', the existing > clients could still write to the db. > > In the case of pacemaker OVN HA, OVN OCF script 'start' action starts the > ovsdb-servers in read-only state and later, it sets to read-write in the > 'promote' action. We have observed that if some ovn-controllers connect to > the SB ovsdb-server (in read-only state) just before the 'promote' action, > the connection is not reset all the times and these ovn-controllers remain connected > to the SB ovsdb-server in read-only state all the time. Even though > the commit [1] calls 'ovsdb_jsonrpc_server_reconnect()' with 'forced' flag > set to true when the db read/write status changes, somehow the FSM misses resetting > the connections of these ovn-controllers. > > I think this needs to be addressed in the FSM. This patch doesn't address > this FSM issue. Instead it changes the behavior of 'ovsdb_jsonrpc_server_set_read_only()' > by setting the 'read_only' flag of all the jsonrpc sessions instead of forcefully > resetting the connection. > > I think there is no need to reset the connection. In large scale production > deployements with OVN, this results in unnecessary waste of CPU cycles as ovn-controllers > will have to connect twice - once during 'start' action and again during 'promote'. > > [1] - 2a9679e3b2c6("ovsdb-server: drop all connections on read/write status change") > > Signed-off-by: Numan Siddique <nusiddiq@redhat.com> > --- > > v1 -> v2 > ------- > * Addressed Dumitru's comment - Use LIST_FOR_EACH instead of > LIST_FOR_EACH_SAFE > > ovsdb/jsonrpc-server.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > index ddbbc2e94..a98c5e618 100644 > --- a/ovsdb/jsonrpc-server.c > +++ b/ovsdb/jsonrpc-server.c > @@ -80,6 +80,8 @@ static void ovsdb_jsonrpc_session_unlock_all(struct ovsdb_jsonrpc_session *); > static void ovsdb_jsonrpc_session_unlock__(struct ovsdb_lock_waiter *); > static void ovsdb_jsonrpc_session_send(struct ovsdb_jsonrpc_session *, > struct jsonrpc_msg *); > +static void ovsdb_jsonrpc_session_set_readonly_all( > + struct ovsdb_jsonrpc_remote *remote, bool read_only); > > /* Triggers. */ > static void ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *, > @@ -365,10 +367,13 @@ 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, true, > - xstrdup(read_only > - ? "making server read-only" > - : "making server read/write")); > + struct shash_node *node; Sorry, I forgot to mention this minor comment earlier.. Could you please move the variable declaration one line before "svr->read_only = read_only;" or add another empty line between the assignment and variable declaration. Except for that: Acked-by: Dumitru Ceara <dceara@redhat.com> > + > + SHASH_FOR_EACH (node, &svr->remotes) { > + struct ovsdb_jsonrpc_remote *remote = node->data; > + > + ovsdb_jsonrpc_session_set_readonly_all(remote, read_only); > + } > } > } > > @@ -670,6 +675,17 @@ ovsdb_jsonrpc_session_reconnect_all(struct ovsdb_jsonrpc_remote *remote, > } > } > > +static void > +ovsdb_jsonrpc_session_set_readonly_all(struct ovsdb_jsonrpc_remote *remote, > + bool read_only) > +{ > + struct ovsdb_jsonrpc_session *s; > + > + LIST_FOR_EACH (s, node, &remote->sessions) { > + s->read_only = read_only; > + } > +} > + > /* Sets the options for all of the JSON-RPC sessions managed by 'remote' to > * 'options'. > * > -- > 2.21.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index ddbbc2e94..a98c5e618 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -80,6 +80,8 @@ static void ovsdb_jsonrpc_session_unlock_all(struct ovsdb_jsonrpc_session *); static void ovsdb_jsonrpc_session_unlock__(struct ovsdb_lock_waiter *); static void ovsdb_jsonrpc_session_send(struct ovsdb_jsonrpc_session *, struct jsonrpc_msg *); +static void ovsdb_jsonrpc_session_set_readonly_all( + struct ovsdb_jsonrpc_remote *remote, bool read_only); /* Triggers. */ static void ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *, @@ -365,10 +367,13 @@ 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, true, - xstrdup(read_only - ? "making server read-only" - : "making server read/write")); + struct shash_node *node; + + SHASH_FOR_EACH (node, &svr->remotes) { + struct ovsdb_jsonrpc_remote *remote = node->data; + + ovsdb_jsonrpc_session_set_readonly_all(remote, read_only); + } } } @@ -670,6 +675,17 @@ ovsdb_jsonrpc_session_reconnect_all(struct ovsdb_jsonrpc_remote *remote, } } +static void +ovsdb_jsonrpc_session_set_readonly_all(struct ovsdb_jsonrpc_remote *remote, + bool read_only) +{ + struct ovsdb_jsonrpc_session *s; + + LIST_FOR_EACH (s, node, &remote->sessions) { + s->read_only = read_only; + } +} + /* Sets the options for all of the JSON-RPC sessions managed by 'remote' to * 'options'. *