On Thu, Oct 3, 2019 at 11:35 AM Ben Pfaff <b...@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 <b...@ovn.org> wrote:
> > >
> > > On Tue, Oct 01, 2019 at 05:05:28PM -0700, amgin...@gmail.com wrote:
> > > > From: Aliasgar Ginwala <aginw...@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 <aginw...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
