Open Stack

On Mon, 13 Aug 2018 at 15:27, Jay Pipes <jaypipes at gmail.com> wrote:
>
> On 08/13/2018 10:10 AM, Matthew Booth wrote:
> > On Mon, 13 Aug 2018 at 14:05, Jay Pipes <jaypipes at gmail.com> wrote:
> >>
> >> On 08/13/2018 06:06 AM, Matthew Booth wrote:
> >>> Thanks mriedem for answering my previous question, and also pointing
> >>> out the related previous spec around just forcing all metadata to be
> >>> lowercase:
> >>>
> >>> (Spec: approved in Newton) https://review.openstack.org/#/c/311529/
> >>> (Online migration: not merged, abandoned)
> >>> https://review.openstack.org/#/c/329737/
> >>>
> >>> There are other code patches, but the above is representative. What I
> >>> had read was the original bug:
> >>>
> >>> https://bugs.launchpad.net/nova/+bug/1538011
> >>>
> >>> The tl;dr is that the default collation used by MySQL results in a bug
> >>> when creating 2 metadata keys which differ only in case. The proposal
> >>> was obviously to simply make all metadata keys lower case. However, as
> >>> melwitt pointed out in the bug at the time that's a potentially user
> >>> hostile change. After some lost IRC discussion it seems that folks
> >>> believed at the time that to fix this properly would seriously
> >>> compromise the performance of these queries. The agreed way forward
> >>> was to allow existing keys to keep their case, but force new keys to
> >>> be lower case (so I wonder how the above online migration came
> >>> about?).
> >>>
> >>> Anyway, as Rajesh's patch shows, it's actually very easy just to fix
> >>> the MySQL misconfiguration:
> >>>
> >>> https://review.openstack.org/#/c/504885/
> >>>
> >>> So my question is, given that the previous series remains potentially
> >>> user hostile, the fix isn't as complex as previously believed, and it
> >>> doesn't involve a performance penalty, are there any other reasons why
> >>> we might want to resurrect it rather than just go with Rajesh's patch?
> >>> Or should we ask Rajesh to expand his patch into a series covering
> >>> other metadata?
> >>
> >> Keep in mind this patch is only related to *aggregate* metadata, AFAICT.
> >
> > Right, but the original bug pointed out that the same problem applies
> > equally to a bunch of different metadata stores. I haven't verified,
> > but the provenance was good ;) There would have to be other patches
> > for the other metadata stores.
>
> Yes, it is quite unfortunate that OpenStack has about 15 different ways
> of storing metadata key/value information.
>
> >>
> >> Any patch series that tries to "fix" this issue needs to include all of
> >> the following:
> >>
> >> * input automatically lower-cased [1]
> >> * inline (note: not online, inline) data migration inside the
> >> InstanceMeta object's _from_db_object() method for existing
> >> non-lowercased keys
> >
> > I suspect I've misunderstood, but I was arguing this is an anti-goal.
> > There's no reason to do this if the db is working correctly, and it
> > would violate the principal of least surprise in dbs with legacy
> > datasets (being all current dbs). These values have always been mixed
> > case, lets just leave them be and fix the db.
>
> Do you want case-insensitive keys or do you not want case-insensitive keys?
>
> It seems to me that people complain that MySQL is case-insensitive by
> default but actually *like* the concept that a metadata key of "abc"
> should be "equal to" a metadata key of "ABC".
>
> In other words, it seems to me that users actually expect that:
>
>  > nova aggregate-create agg1
>  > nova aggregate-set-metadata agg1 abc=1
>  > nova aggregate-set-metadata agg1 ABC=2
>
> should result in the original "abc" metadata item getting its value set
> to "2".
>
> If that isn't the case -- and I have a very different impression of what
> users *actually* expect from the CLI/UI -- then let me know.

I don't know what users want, tbh, I was simply coming from the POV of
not breaking the current behaviour. Although I think you're pointing
out that either solution breaks the current behaviour:

1. You lower case everything. This breaks users who query user
metadata and don't expect keys to be modified.

2. You fix the case sensitivity. This breaks users who add 'Foo' and
now expect to query 'foo'.

You're saying that although (2) is an artifact of a bug, there could
equally be people relying on it.

Eurgh. Yeah, that sucks.

Objectively though, I think I still like Rajesh's patch better because:

* It's vastly simpler to implement correctly and verifiably, and
therefore also less prone to future bugs.
* It's how it was originally intended to work.
* It's simpler to document.

Of these, the first is by far the most persuasive.

Matt
-- 
Matthew Booth
Red Hat OpenStack Engineer, Compute DFG

Phone: +442070094448 (UK)