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 <[email protected]> wrote: > On Thu, Oct 3, 2019 at 11:35 AM Ben Pfaff <[email protected]> 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 <[email protected]> wrote: > > > > > > > > On Tue, Oct 01, 2019 at 05:05:28PM -0700, [email protected] wrote: > > > > > From: Aliasgar Ginwala <[email protected]> > > > > > > > > > > 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 <[email protected]> > > > > > > > > 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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
