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

Ben Pfaff blp at ovn.org
Thu Oct 3 18:35:15 UTC 2019


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; }".

> 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.


More information about the dev mailing list