> On Feb. 18, 2016, 10:32 p.m., Adam B wrote: > > include/mesos/mesos.proto, line 1600 > > <https://reviews.apache.org/r/43616/diff/2/?file=1252884#file1252884line1600> > > > > Is it ok for labels to contain duplicate keys even if the values are > > different? > > That sounds like undefined behavior too. Is the label-consumer supposed > > to use all values, first value, last value? > > Neil Conway wrote: > To me, the issue isn't how the label-consumer is supposed to interpret > the labels: rather, labels with duplicate key-value pairs are not handled > correctly by Mesos (our equality operator is wrong for this situation -- see > MESOS-4445). The initial feeling was that the runtime cost of fixing the > equality operator wasn't worth it (although based on the experiments in > https://reviews.apache.org/r/43686/, it is unclear whether this is true). > > The equality operator behaves correctly for labels with duplicate keys > but distinct values associated with those keys. How label consumers are > supposed to interpret them is up to the application.
I see. You're working on preventing incorrect behavior first, rather than worrying about preventing (or defining) undefined behavior. Give them a shorter noose. Works for me. We can clarify semantics for duplicate keys later. > On Feb. 18, 2016, 10:32 p.m., Adam B wrote: > > include/mesos/mesos.proto, line 1634 > > <https://reviews.apache.org/r/43616/diff/2/?file=1252884#file1252884line1634> > > > > Are you intentionally leaving out the Labels field in DiscoveryInfo, > > Port, NetworkInfo, Image.Appc? I didn't think Mesos interpreted any of > > these either. > > Neil Conway wrote: > It just seemed a bit verbose to copy the same comment that many times, so > I settled for documenting the most common use-sites as well as the definition > of `Labels`. Happy to change that if people would rather see it done another > way though. Yeah, I agree that the only necessary one is the root Labels message, and the other comments are bonus. Just wanted to make sure it was a conscious choice. Dropping the issue. - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43616/#review119822 ----------------------------------------------------------- On Feb. 17, 2016, 10:44 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43616/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2016, 10:44 a.m.) > > > Review request for mesos and Michael Park. > > > Repository: mesos > > > Description > ------- > > The implementation of the equality operator for `Labels` is buggy for labels > that contain duplicates. We might want to revisit fixing the implementation of > that operator (which might be expensive; MESOS-4445), but in the short-term we > should document that duplicates should not be specified. > > > Diffs > ----- > > include/mesos/mesos.proto e24d3e03a7dc7c6bfd07f34531cb593fe4925646 > include/mesos/v1/mesos.proto d909e60ddfd8e3ba2075f82c372edde04cd99d54 > > Diff: https://reviews.apache.org/r/43616/diff/ > > > Testing > ------- > > > Thanks, > > Neil Conway > >
