Re: Review Request 50836: Made add/subtract resource object as private method.

2016-08-30 Thread Guangya Liu
> On 八月 16, 2016, 11:44 p.m., Jiang Yan Xu wrote: > > include/mesos/resources.hpp, line 59 > > > > > > I think it would still work if you keep the `static` keyword right? > > > > The `friend` keyword

Re: Review Request 50836: Made add/subtract resource object as private method.

2016-08-30 Thread Jiang Yan Xu
> On Aug. 16, 2016, 4:44 p.m., Jiang Yan Xu wrote: > > include/mesos/resources.hpp, line 59 > > > > > > I think it would still work if you keep the `static` keyword right? > > > > The `friend` keyword

Re: Review Request 51402: Added nested container check in provisioner destroy.

2016-08-30 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51402/#review147381 --- src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines

Re: Review Request 51392: Supported provisioner listContainers() to be recursive.

2016-08-30 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51392/#review147380 --- src/slave/containerizer/mesos/provisioner/paths.cpp (lines 151 -

Re: Review Request 50994: Added 'HealthCheck' protobuf validation.

2016-08-30 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50994/#review147376 --- src/health-check/health_checker.cpp (lines 290 - 292)

Re: Review Request 51529: Initialize sorter pointers in HierarchicalAllocatorProcess constructor.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51529/#review147371 --- Patch looks great! Reviews applied: [51528, 51529] Passed

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

2016-08-30 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51358/#review147368 --- src/slave/containerizer/mesos/provisioner/paths.cpp (line 122)

Re: Review Request 51343: Refactored the redundant logic in provisioner recover().

2016-08-30 Thread Joseph Wu
> On Aug. 25, 2016, 10:57 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 204 > > > > > > Why this change? Where is the error checking? > > Gilbert Song wrote: > I

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Jiang Yan Xu
> On Aug. 30, 2016, 12:18 p.m., Benjamin Bannier wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 2695 > > > > > > I had to look up whether `HierarchicalAllocatorTestBase::initialize` is > > called

Re: Review Request 51529: Initialize sorter pointers in HierarchicalAllocatorProcess constructor.

2016-08-30 Thread Jiang Yan Xu
> On Aug. 30, 2016, 12:24 p.m., Benjamin Bannier wrote: > > Thanks for the review! > On Aug. 30, 2016, 12:24 p.m., Benjamin Bannier wrote: > > src/master/allocator/mesos/hierarchical.hpp, line 465 > > > > > >

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs: WIP.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51052/#review147352 --- Patch looks great! Reviews applied: [51052] Passed command:

Re: Review Request 51529: Initialize sorter pointers in HierarchicalAllocatorProcess constructor.

2016-08-30 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51529/#review147336 --- Fix it, then Ship it!

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51528/#review147339 --- Fix it, then Ship it!

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Jiang Yan Xu
> On Aug. 30, 2016, 11:41 a.m., Guangya Liu wrote: > > @Jiang Yan, what about switch the order of this and > > https://reviews.apache.org/r/51529/ so as to make sure the build will not > > be failed after this merged? > > Guangya Liu wrote: > Sorry, I mean the unit test will failed if

Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/#review147333 --- Patch looks great! Reviews applied: [51028, 51027] Passed

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Guangya Liu
> On 八月 30, 2016, 6:41 p.m., Guangya Liu wrote: > > @Jiang Yan, what about switch the order of this and > > https://reviews.apache.org/r/51529/ so as to make sure the build will not > > be failed after this merged? Sorry, I mean the unit test will failed if keeping such order. - Guangya

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51528/#review147331 --- @Jiang Yan, what about switch the order of this and

Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-30 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/#review147321 --- src/master/allocator/mesos/hierarchical.hpp (line 219)

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs: WIP.

2016-08-30 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51052/ --- (Updated Aug. 30, 2016, 5:56 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51528/ --- (Updated Aug. 30, 2016, 10:07 a.m.) Review request for mesos, Alexander

Review Request 51528: Added 'HierarchicalAllocatorTest.ResourceMetricsUninitialized' test.

2016-08-30 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51528/ --- Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Guangya Liu,

Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-30 Thread Jacob Janco
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/ --- (Updated Aug. 30, 2016, 4:50 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-30 Thread Jacob Janco
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/ --- (Updated Aug. 30, 2016, 4:53 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 51525: Added tooling to execute Mesos-specific clang-tidy checks.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51525/#review147312 --- Patch looks great! Reviews applied: [51525] Passed command:

Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-30 Thread Jacob Janco
> On Aug. 26, 2016, 6:24 a.m., Klaus Ma wrote: > > src/master/allocator/mesos/hierarchical.cpp, line 275 > > > > > > I think we should use `insert(...)` instead of `=`; if the following > > event in the queue, are

Re: Review Request 50661: Documentation: fix broken links.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50661/#review147299 --- Patch looks great! Reviews applied: [50661] Passed command:

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

2016-08-30 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46824/ --- (Updated Aug. 30, 2016, 4:36 p.m.) Review request for mesos, Alexander

Re: Review Request 50661: Documentation: fix broken links.

2016-08-30 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50661/#review147293 --- Fix it, then Ship it! Verified by re generate the website, the

Re: Review Request 51477: Implemented `RunTaskGroupMessage` handler on the agent.

2016-08-30 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51477/#review147294 --- src/slave/slave.cpp (lines 1513 - 1517)

Review Request 51525: Added tooling to execute Mesos-specific clang-tidy checks.

2016-08-30 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51525/ --- Review request for mesos and Michael Park. Bugs: MESOS-4907

Re: Review Request 50661: Documentation: fix broken links.

2016-08-30 Thread Pierre Cheynier
> On Aug. 25, 2016, 8:48 p.m., Vinod Kone wrote: > > Pierre, still working on this? If yes, can you please address issues raised > > by haosdent? If not, please discard the review. Yes, I'm sorry about that, I was off for a long time. I published an update to address the issues opened by

Re: Review Request 50661: Documentation: fix broken links.

2016-08-30 Thread Pierre Cheynier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50661/ --- (Updated Aug. 30, 2016, 1:52 p.m.) Review request for mesos, Dave Lester,

Re: Review Request 51520: Update contributors.yaml for gradywang

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51520/#review147290 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 51517: Added MOUNT or PATH disk type in logging resources.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51517/#review147288 --- Patch looks great! Reviews applied: [51517] Passed command:

Review Request 51520: Update contributors.yaml for gradywang

2016-08-30 Thread Yongqiao Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51520/ --- Review request for mesos. Repository: mesos Description --- Update

Review Request 51517: Added MOUNT or PATH disk type in logging resources.

2016-08-30 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51517/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-6060

Re: Review Request 51470: Add the 'resources' field back to DRFSorter::Total.

2016-08-30 Thread Anindya Sinha
> On Aug. 29, 2016, 7:26 a.m., Anindya Sinha wrote: > > I think we should also bring back this function: > > `const hashmap& total()`. > > Jiang Yan Xu wrote: > Why? The total is now fully hidden and we only need to reply on it within > DRFSorter for shared resources. >

Re: Review Request 46187: Terminate when receiving the ACK of terminal status update.

2016-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46187/#review147277 --- Patch looks great! Reviews applied: [46187] Passed command:

Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.

2016-08-30 Thread Guangya Liu
> On 八月 25, 2016, 10:56 p.m., Jiang Yan Xu wrote: > > I don't know. Making such distinction feels like splitting hairs to me. > > Yeah there are subtle differences between the two concepts but to me: > > > > - `offerCallback` may not be the best name what we could have come up with > > even

Re: Review Request 46187: Terminate when receiving the ACK of terminal status update.

2016-08-30 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46187/ --- (Updated Aug. 30, 2016, 2:01 p.m.) Review request for mesos, Anand Mazumdar