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

Ben Pfaff blp at ovn.org
Fri Oct 4 17:38:55 UTC 2019


OK.

I've made the points that I consider relevant.  I'll let the rest of the
OVN community come to consensus here.

On Thu, Oct 03, 2019 at 02:17:48PM -0700, aginwala wrote:
> 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