You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/04/28 00:43:42 UTC
Review Request 58828: Replaced std::set with hashset for framework
roles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58828/
-----------------------------------------------------------
Review request for mesos and Michael Park.
Repository: mesos
Description
-------
Since we do not need ordering, this updates the framework roles to
be stored using hashset instead of std::set.
Diffs
-----
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/hierarchical.hpp 219f508dacb3b372c12da3f15e07a3bf5f27e6e6
src/master/allocator/mesos/hierarchical.cpp 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa
src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
src/master/master.cpp a0cb505a1300637f42fa8062c57deb1590779dd4
src/slave/slave.cpp 8b8078dbb656e9db2efa53cc4ec5bed2cc01d49a
src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328
Diff: https://reviews.apache.org/r/58828/diff/1/
Testing
-------
make check
Thanks,
Benjamin Mahler
Re: Review Request 58828: Replaced std::set with hashset for
framework roles.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58828/#review173302
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On April 27, 2017, 5:43 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58828/
> -----------------------------------------------------------
>
> (Updated April 27, 2017, 5:43 p.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since we do not need ordering, this updates the framework roles to
> be stored using hashset instead of std::set.
>
>
> Diffs
> -----
>
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/hierarchical.hpp 219f508dacb3b372c12da3f15e07a3bf5f27e6e6
> src/master/allocator/mesos/hierarchical.cpp 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa
> src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
> src/master/master.cpp a0cb505a1300637f42fa8062c57deb1590779dd4
> src/slave/slave.cpp 8b8078dbb656e9db2efa53cc4ec5bed2cc01d49a
> src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328
>
>
> Diff: https://reviews.apache.org/r/58828/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 58828: Replaced std::set with hashset for
framework roles.
Posted by Benjamin Mahler <bm...@apache.org>.
> On April 28, 2017, 6:30 a.m., Michael Park wrote:
> > It's true that we don't use `std::set` for its ordering in the general case,
> > however, I think the ordering does help produce easier-to-read error messages.
> >
> >
> > What do you think of preserving that property?
Yes you're right, I forgot that was the original reason I suggested std::set, given we log it. My intention with this change was to order it when we need to (before logging it), but seems it is not worth the trouble, I'll drop this.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58828/#review173299
-----------------------------------------------------------
On April 28, 2017, 12:43 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58828/
> -----------------------------------------------------------
>
> (Updated April 28, 2017, 12:43 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since we do not need ordering, this updates the framework roles to
> be stored using hashset instead of std::set.
>
>
> Diffs
> -----
>
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/hierarchical.hpp 219f508dacb3b372c12da3f15e07a3bf5f27e6e6
> src/master/allocator/mesos/hierarchical.cpp 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa
> src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
> src/master/master.cpp a0cb505a1300637f42fa8062c57deb1590779dd4
> src/slave/slave.cpp 8b8078dbb656e9db2efa53cc4ec5bed2cc01d49a
> src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328
>
>
> Diff: https://reviews.apache.org/r/58828/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 58828: Replaced std::set with hashset for
framework roles.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58828/#review173299
-----------------------------------------------------------
It's true that we don't use `std::set` for its ordering in the general case,
however, I think the ordering does help produce easier-to-read error messages.
What do you think of preserving that property?
- Michael Park
On April 27, 2017, 5:43 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58828/
> -----------------------------------------------------------
>
> (Updated April 27, 2017, 5:43 p.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since we do not need ordering, this updates the framework roles to
> be stored using hashset instead of std::set.
>
>
> Diffs
> -----
>
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/hierarchical.hpp 219f508dacb3b372c12da3f15e07a3bf5f27e6e6
> src/master/allocator/mesos/hierarchical.cpp 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa
> src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
> src/master/master.cpp a0cb505a1300637f42fa8062c57deb1590779dd4
> src/slave/slave.cpp 8b8078dbb656e9db2efa53cc4ec5bed2cc01d49a
> src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328
>
>
> Diff: https://reviews.apache.org/r/58828/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>