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

Han Zhou zhouhan at gmail.com
Thu Oct 3 19:38:11 UTC 2019


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?

Thanks,
Han


More information about the dev mailing list