> 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
> 
>

Reply via email to