Re: Review Request 42688: Fixed equality semantics for Labels.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42688/#review119248 --- Per comments in MESOS-4445, the current plan is to not change equality behavior right now, and just to document that duplicates in `Labels` are not supported. - Neil Conway On Jan. 23, 2016, 8:04 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42688/ > --- > > (Updated Jan. 23, 2016, 8:04 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-4445 > https://issues.apache.org/jira/browse/MESOS-4445 > > > Repository: mesos > > > Description > --- > > The previous implementation was buggy in two scenarios: (1) the label sets > contained different #s of duplicate elements (2) the "left" label set > contained > duplicates and the "right" label set contained a value that didn't appear in > the > "left" set. > > > Diffs > - > > src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f > src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 > src/tests/labels_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/42688/diff/ > > > Testing > --- > > make check > > REVIEW NOTES: > > * This is a WIP. > * Rather than implement a correct equality algorithm myself, I decided to > just use `std::unordered_multiset`. Computing equality correctly is a bit > involved. > * This requires a hash implementation for `Label`. I supplied this in new > function object in the `mesos` namespace, but we could also supply a > specialization of `std::hash`. Not sure which is better. > * The same bug occurs in other equality operators for repeated fields > (`CommandInfo.uris`, `Environment`, `DockerInfo`, etc.). I suppose we should > probably fix these as well? Note that the bugs in the old approach are a lot > less likely to occur when the struct has a lot of fields. > > > Thanks, > > Neil Conway > >
Review Request 42688: Fixed equality semantics for Labels.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42688/ --- Review request for mesos and Michael Park. Bugs: MESOS-4445 https://issues.apache.org/jira/browse/MESOS-4445 Repository: mesos Description --- The previous implementation was buggy in two scenarios: (1) the label sets contained different #s of duplicate elements (2) the "left" label set contained duplicates and the "right" label set contained a value that didn't appear in the "left" set. Diffs - src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 src/tests/labels_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/42688/diff/ Testing --- make check REVIEW NOTES: * This is a WIP. * Rather than implement a correct equality algorithm myself, I decided to just use `std::unordered_multiset`. Computing equality correctly is a bit involved. * This requires a hash implementation for `Label`. I supplied this in new function object in the `mesos` namespace, but we could also supply a specialization of `std::hash`. Not sure which is better. * The same bug occurs in other equality operators for repeated fields (`CommandInfo.uris`, `Environment`, `DockerInfo`, etc.). I suppose we should probably fix these as well? Note that the bugs in the old approach are a lot less likely to occur when the struct has a lot of fields. Thanks, Neil Conway
Re: Review Request 42688: Fixed equality semantics for Labels.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42688/#review115999 --- Patch looks great! Reviews applied: [42688] Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh - Mesos ReviewBot On Jan. 23, 2016, 8:04 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42688/ > --- > > (Updated Jan. 23, 2016, 8:04 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-4445 > https://issues.apache.org/jira/browse/MESOS-4445 > > > Repository: mesos > > > Description > --- > > The previous implementation was buggy in two scenarios: (1) the label sets > contained different #s of duplicate elements (2) the "left" label set > contained > duplicates and the "right" label set contained a value that didn't appear in > the > "left" set. > > > Diffs > - > > src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f > src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 > src/tests/labels_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/42688/diff/ > > > Testing > --- > > make check > > REVIEW NOTES: > > * This is a WIP. > * Rather than implement a correct equality algorithm myself, I decided to > just use `std::unordered_multiset`. Computing equality correctly is a bit > involved. > * This requires a hash implementation for `Label`. I supplied this in new > function object in the `mesos` namespace, but we could also supply a > specialization of `std::hash`. Not sure which is better. > * The same bug occurs in other equality operators for repeated fields > (`CommandInfo.uris`, `Environment`, `DockerInfo`, etc.). I suppose we should > probably fix these as well? Note that the bugs in the old approach are a lot > less likely to occur when the struct has a lot of fields. > > > Thanks, > > Neil Conway > >
