[ovs-dev] [PATCH v1 ovn] ovn-nb/sbctl.c: Set no-leader-only as default for clustered dbs

Han Zhou zhouhan at gmail.com
Wed Oct 2 19:25:10 UTC 2019


On Tue, Oct 1, 2019 at 5:06 PM <amginwal at gmail.com> wrote:
>
> From: Aliasgar Ginwala <aginwala at ebay.com>
>
> When using ovn-nb/sbctl running in cluster, one can use local
> socket to run different commands. It is very inconvenient to pass
> no-leader-only in different tools using ovn-nb/sbctl instead of
> allowing one to to connect to any nodes in the cluster including
> itself.
> e.g common usage ovn-nb/sbctl show.
> Hence, this commit handles the same.
>
> Signed-off-by: Aliasgar Ginwala <aginwala at ebay.com>
> ---
>  utilities/ovn-nbctl.8.xml | 17 +++++++++--------
>  utilities/ovn-nbctl.c     |  3 ++-
>  utilities/ovn-sbctl.8.in  | 18 +++++++++---------
>  utilities/ovn-sbctl.c     |  2 +-
>  4 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index fd75c0e44..3dd05fa65 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1192,14 +1192,15 @@
>      <dt><code>--leader-only</code></dt>
>      <dt><code>--no-leader-only</code></dt>
>      <dd>
> -      By default, or with <code>--leader-only</code>, when the database
server
> -      is a clustered database, <code>ovn-nbctl</code> will avoid servers
other
> -      than the cluster leader.  This ensures that any data that
> -      <code>ovn-nbctl</code> reads and reports is up-to-date.  With
> -      <code>--no-leader-only</code>, <code>ovn-nbctl</code> will use any
server
> -      in the cluster, which means that for read-only transactions it can
report
> -      and act on stale data (transactions that modify the database are
always
> -      serialized even with <code>--no-leader-only</code>).  Refer to
> +      By default, or with <code>--no-leader-only</code>, when the
database
> +      server is a clustered database, <code>ovn-nbctl</code> may connect
to
> +      any server in the cluster, which means that for read-only
transactions
> +      it can report and act on stale data (transactions that modify the
> +      database are always serialized even with
<code>--no-leader-only</code>).
> +      To avoid reading stale data, one can specify
<code>--leader-only</code>,
> +      so that <code>ovn-nbctl</code> will avoid servers other than the
cluster
> +      leader. For daemon mode, since it is for long running connections,
> +      default is set to <code>--leader-only</code>. Refer to
>        <code>Understanding Cluster Consistency</code> in
<code>ovsdb</code>(7)
>        for more information.
>      </dd>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index a89a9cb4d..3804dd25a 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -81,7 +81,7 @@ static struct ovsdb_idl_txn *the_idl_txn;
>  OVS_NO_RETURN static void nbctl_exit(int status);
>
>  /* --leader-only, --no-leader-only: Only accept the leader in a cluster.
*/
> -static int leader_only = true;
> +static int leader_only = false;
>
>  /* --shuffle-remotes, --no-shuffle-remotes: Shuffle the order of remotes
that
>   * are specified in the connetion method string. */
> @@ -188,6 +188,7 @@ main(int argc, char *argv[])
>                        "(use --help for help)");
>          }
>          daemon_mode = true;
> +        leader_only = true;
>      }
>      /* Initialize IDL. */
>      idl = the_idl = ovsdb_idl_create_unconnected(&nbrec_idl_class, true);
> diff --git a/utilities/ovn-sbctl.8.in b/utilities/ovn-sbctl.8.in
> index 2aaa457e8..f52412812 100644
> --- a/utilities/ovn-sbctl.8.in
> +++ b/utilities/ovn-sbctl.8.in
> @@ -53,16 +53,16 @@ e.g. \fBssl:192.168.10.5:6640\fR, as described in
\fBovsdb\fR(7).
>  .
>  .IP "\fB\-\-leader\-only\fR"
>  .IQ "\fB\-\-no\-leader\-only\fR"
> -By default, or with \fB\-\-leader\-only\fR, when the database server
> -is a clustered database, \fBovn\-sbctl\fR will avoid servers other
> -than the cluster leader.  This ensures that any data that
> -\fBovn\-sbctl\fR reads and reports is up-to-date.  With
> -\fB\-\-no\-leader\-only\fR, \fBovn\-sbctl\fR will use any server in
> -the cluster, which means that for read-only transactions it can report
> +By default, or with \fB\-\-no\-leader\-only\fR, when the database server
> +is a clustered database, \fBovn\-sbctl\fR may connect to any server
> +in the cluster, which means that for read-only transactions it can report
>  and act on stale data (transactions that modify the database are
> -always serialized even with \fB\-\-no\-leader\-only\fR).  Refer to
> -\fBUnderstanding Cluster Consistency\fR in \fBovsdb\fR(7) for more
> -information.
> +always serialized even with \fB\-\-no\-leader\-only\fR). To avoid reading
> +stale data, one can specify \fB\-\-leader\-only\fR, so that
> +\fBovn\-sbctl\fR will avoid servers other than the cluster leader. For
daemon
> +mode, since it is for long running connections, default is set to
> +\fB\-\-leader\-only\fR. Refer to \fBUnderstanding Cluster Consistency\fR
in
> +\fBovsdb\fR(7) for more information.
>  .
>  .IP "\fB\-\-no\-syslog\fR"
>  By default, \fBovn\-sbctl\fR logs its arguments and the details of any
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index 9a9b6f0ec..f1cb8790f 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -82,7 +82,7 @@ static struct ovsdb_idl_txn *the_idl_txn;
>  OVS_NO_RETURN static void sbctl_exit(int status);
>
>  /* --leader-only, --no-leader-only: Only accept the leader in a cluster.
*/
> -static int leader_only = true;
> +static int leader_only = false;
>
>  static void sbctl_cmd_init(void);
>  OVS_NO_RETURN static void usage(void);
> --
> 2.20.1 (Apple Git-117)
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Aliasgar.
Acked-by: Han Zhou <hzhou8 at ebay.com>

I'd leave to Ben and others to take a look and confirm. I support this
change because based on the day-to-day usage/test it seems more convenient
this way. We'd better change this before there are too many users of
clustered mode.


More information about the dev mailing list