On Fri, May 29, 2020 at 10:42 PM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
>
> On Fri, May 29, 2020 at 9:10 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >
> > On Fri, 2020-05-29 at 20:38 +0200, Ilya Dryomov wrote:
> > > On Fri, May 29, 2020 at 7:27 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > > > On Fri, 2020-05-29 at 17:19 +0200, Ilya Dryomov wrote:
> > > > > Allow expressing client's location in terms of CRUSH hierarchy as
> > > > > a set of (bucket type name, bucket name) pairs. The userspace syntax
> > > > > "crush_location = key1=value1 key2=value2" is incompatible with mount
> > > > > options and needed adaptation:
> > > > >
> > > > > - ':' separator
> > > > > - one key:value pair per crush_location option
> > > > > - crush_location options are combined together
> > > > >
> > > > > So for:
> > > > >
> > > > > crush_location = host=foo rack=bar
> > > > >
> > > > > one would write:
> > > > >
> > > > > crush_location=host:foo,crush_location=rack:bar
> > > > >
> > > > > As in userspace, "multipath" locations are supported, so indicating
> > > > > locality for parallel hierarchies is possible:
> > > > >
> > > > > crush_location=rack:foo1,crush_location=rack:foo2,crush_location=datacenter:bar
> > > > >
> > > >
> > > > Blech, that syntax is hideous. It's also problematic in that the options
> > > > are additive -- you can't override an option that was given earlier
> > > > (e.g. in fstab), or in a shell script.
> > > >
> > > > Is it not possible to do something with a single crush_location= option?
> > > > Maybe:
> > > >
> > > > crush_location=rack:foo1/rack:foo2/datacenter:bar
> > > >
> > > > It's still ugly with the embedded '=' signs, but it would at least make
> > > > it so that the options aren't additive.
> > >
> > > I suppose we could do something like that at the cost of more
> > > parsing boilerplate, but I'm not sure additive options are that
> > > hideous. I don't think additive options are unprecedented and
> > > more importantly I think many simple boolean and integer options
> > > are not properly overridable even in major filesystems.
> > >
> >
> > That is the long-standing convention though. There are reasons to
> > deviate from it, but I don't see it here. Plus, I think the syntax I
> > proposed above is more readable (and compact) as well.
> >
> > It would mean a bit more parsing code though, granted.
> >
> > > What embedded '=' signs are you referring to? I see ':' and '/'
> > > in your suggested syntax.
> > >
> >
> > Sorry, yeah... I had originally done one that had '=' chars in it, but
> > converted it to the above. Please disregard that paragraph.
>
> One of the reasons I did it this way is that crush_location is
> inherently additive. I don't have a strong opinion on this though
> so let's adhere to the convention.
>
> I'll implement the suggested syntax and repost.
I went with '|' instead of '/' for the separator to try to stress
the additivity (in the OR sense). '/' makes it look like a path to
the root of the tree which it really isn't.
Thanks,
Ilya
