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

aginwala aginwala at asu.edu
Thu Oct 3 21:17:48 UTC 2019


Thanks Han and ben for the suggestions. However, one more reason for using
this approach is because ovn-controller uses no-leader-only by default so
that all the chassis can be randomly distributed to talk to any node in the
cluster to avoid overloading leader node in a large scale env
"ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false)"

On Thu, Oct 3, 2019 at 12:38 PM Han Zhou <zhouhan at gmail.com> wrote:

> On Thu, Oct 3, 2019 at 11:35 AM Ben Pfaff <blp at ovn.org> wrote:
> >
> > On Thu, Oct 03, 2019 at 10:19:35AM -0700, Han Zhou wrote:
> > > On Thu, Oct 3, 2019 at 9:09 AM Ben Pfaff <blp at ovn.org> wrote:
> > > >
> > > > On Tue, Oct 01, 2019 at 05:05:28PM -0700, 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>
> > > >
> > > > This change makes more of a difference than its size implies, because
> it
> > > > means that scripts that previously were guaranteed to get up-to-date
> > > > data can now get inconsistent results.  It loses read-after-write
> > > > consistency, for example.  I'd really prefer to avoid surprising
> users
> > > > with that kind of thing (especially as a change).
> > > >
> > > > If it's common to want that kind of behavior, though, perhaps there
> > > > could be a nice way to set it as the default for a session.  In
> daemon
> > > > mode, of course, it's already possible to control it for the daemon's
> > > > users.  One option for outside daemon mode might be to introduce an
> > > > environment variable.  The environment variable could be specific to
> > > > this feature, e.g. OVN_LEADER_ONLY=0 or OVN_LEADER_ONLY=1, or it
> could
> > > > be a general-purpose options variable,
> > > > e.g. OVN_OPTIONS=--no-leader-only.  I don't know whether it should be
> > > > specific to one of ovn-sbctl and ovn-nbctl or apply to both.
> > > >
> > > > Have you thought about these possibilities?
> > > >
> > > Thanks Ben for the comments. Let's compare the pros and cons of each
> > > solution:
> >
> > Thank you for listing the pros and cons.  I agree with them.
> >
> > One factor that is not completely captured by your list is the effect on
> > transitioning deployments from standalone to clustered.  Currently, this
> > should not have surprising effects: one would just replace the single
> > database URL by a comma-separated list and it will continue to work,
> > just more reliably.  If the default changes to --no-leader-only, then
> > each use of ovn-nbctl and ovn-sbctl in the deployment requires some
> > thought.
> >
> > I have one comment that's really about the patch rather than the pros
> > and cons, but I didn't mention it earlier and I should have.  It's
> > related to this.
> >
> > > cons: There are *small* chances that inconsistent (stale) data can be
> read
> > > by the existing tools (except daemon mode, because this patch enforces
> > > leader-only as default for daemon mode). [...]
> >
> > I believe the patch does something like "daemon_mode = true; leader_only
> > = true;" for --daemon-mode.  I think this is a bad idea because it means
> > that "--no-leader-only --daemon-mode" will not have the intended
> > effect.  Instead, we should start out with "leader_only = -1;" and then
> > after parsing options do something like "if (leader_only < 0) {
> > leader_only = daemon_mode; }".
> >
> Thanks for the finding!
>
> > > I just listed the pros and cons for discussion. I don't have a strong
> > > opinion which way is the best. (The number of pros/cons doesn't
> necessarily
> > > mean it is better/worse)
> >
> > I don't have a strong opinion either, but I want to make sure that the
> > implications receive some discussion.
>
> Overall, the --no-leader-only is for convenience. In most cases when
> executing ovn-nb/sbctl it is read-only using default OVN_NB/SB_DB which is
> the unix socket. Even when using TCP/SSL, when it is read-only we'd prefer
> --no-leader-only because it doesn't need to retry until finding the leader.
> The --leader-only is for correctness/safety. We don't want any surprise
> when user should use --leader-only but forget to.
>
> So, we should make the decision based on whether we want the default
> behavior to be the most convenient one or the safest one. I guess if we
> have to choose, the safety should be more important. So I tend to agree
> with the 3rd option as suggested by Ben, which by default ensures safety,
> while providing a way for user to change the default behavior when he/she
> decides to. It is a compromise between the two other options. I would
> propose different environment variable names OVN_NBCTL_OPTIONS and
> OVN_SBCTL_OPTIONS, to make the them more specific to the command but
> general enough for adjusting default behavior for any options. What do you
> think?
>
I am ok with the env variable change if that's the final
conclusion.However, if we are changing for both nb/sb-ctl , I think just
OVN_OPTIONS is ok too instead of setting two env vars. Let me know and I
can send v2 accordingly.

>
> Thanks,
> Han
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list