Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/utils.cpp > > Lines 157-158 (original), 155-156 (patched) > > > > > > Sorry I overlooked this in my last around of re

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55895/#review175768 --- Ship it! Ship It! - Jiang Yan Xu On May 16, 2017, 4:09 p.m.,

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/utils.cpp > > Lines 157-158 (original), 155-156 (patched) > > > > > > Sorry I overlooked this in my last around of re

Re: Review Request 59355: Optimized sorter performance with many inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59355/#review175781 --- Patch looks great! Reviews applied: [59481, 59482, 59383, 59483,

Re: Review Request 59101: Enabled authorization for v1 API call GET_MAINTENANCE_STATUS.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59101/ --- (Updated May 23, 2017, 3:55 p.m.) Review request for mesos, Adam B and Greg Man

Re: Review Request 59147: Enabled authorization for v1 calls starting and stopping maintenance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59147/ --- (Updated May 23, 2017, 3:57 p.m.) Review request for mesos, Adam B and Greg Man

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59463/#review175714 --- src/tests/slave_recovery_tests.cpp Lines 870 (patched)

Re: Review Request 59481: Added sorter test for allocation queries about inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59481/#review175795 --- Ship it! I would find it helpful to have a summary of what this

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175800 --- src/cli_new/lib/cli/tests/base.py Lines 52 (patched)

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53840/ --- (Updated May 23, 2017, 4:30 p.m.) Review request for mesos, James Peach and Jia

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53842/ --- (Updated May 23, 2017, 4:30 p.m.) Review request for mesos, James Peach and Jia

Re: Review Request 53841: Added metrics for sorting in the role and quota sorters.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53841/ --- (Updated May 23, 2017, 4:30 p.m.) Review request for mesos, James Peach and Jia

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175802 --- src/cli_new/lib/cli/tests/base.py Lines 198 (patched)

Re: Review Request 59470: Windows: Fixed broken build due to environment secret isolator.

> On May 22, 2017, 11:32 p.m., Mesos Reviewbot Windows wrote: > > Bad patch! > > > > Reviews applied: [59470] > > > > Failed command: ['cmd', '/c', 'support\\windows-build.bat 2>&1 > > > build_59470'] > > > > Error: > > > > C:\Users\mesos\mesos>REM Licensed to the Apache Software Foundation

Re: Review Request 59355: Optimized sorter performance with many inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59355/#review175812 --- src/master/allocator/sorter/drf/sorter.hpp Lines 316 (patched)

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175813 --- src/cli_new/lib/cli/tests/base.py Lines 333 (patched)

Re: Review Request 59147: Enabled authorization for v1 calls starting and stopping maintenance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59147/#review175814 --- Patch looks great! Reviews applied: [59453, 58955, 58964, 59099,

Re: Review Request 59355: Optimized sorter performance with many inactive clients.

> On May 23, 2017, 4:56 p.m., James Peach wrote: > > src/master/allocator/sorter/drf/sorter.hpp > > Lines 316 (patched) > > > > > > Did you consider keeping separate vectors for active and inactive > > children? Tha

Re: Review Request 59453: Renamed RegisterAgent.agent to RegisterAgent.agents in acls.proto.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59453/#review175820 --- Fix it, then Ship it! src/authorizer/local/authorizer.cpp Line

Re: Review Request 59015: Implemented passing docker config depended methods.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59015/ --- (Updated May 23, 2017, 10:54 a.m.) Review request for mesos, Adam B, Chun-Hung

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175826 --- src/cli_new/lib/cli/tests/base.py Lines 265 (patched)

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175828 --- src/cli_new/lib/cli/tests/base.py Lines 263 (patched)

Re: Review Request 49571: Added a benchmark test for allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49571/ --- (Updated May 23, 2017, 6:59 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53842/#review175831 --- Patch looks great! Reviews applied: [53840, 53841, 53842] Passed

Re: Review Request 59147: Enabled authorization for v1 calls starting and stopping maintenance.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59147/#review175832 --- Patch looks great! Reviews applied: [59453, 58955, 58964, 59099,

Re: Review Request 59015: Implemented passing docker config depended methods.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59015/#review175833 --- Ship it! Ship It! - Vinod Kone On May 23, 2017, 5:54 p.m., G

Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59141/#review175834 --- Fix it, then Ship it! include/mesos/uri/fetcher.hpp Line 72 (o

Re: Review Request 59017: Added support for docker spec helper 'parseAuthConfig()'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59017/#review175835 --- Ship it! - Vinod Kone On May 22, 2017, 6:03 p.m., Gilbert Son

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59018/#review175836 --- Fix it, then Ship it! src/uri/fetchers/docker.cpp Lines 538 (p

Re: Review Request 59428: Moved a couple of logging lines to `INFO` level.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59428/#review175837 --- Do we typically do `LOG(INFO)` in libprocess? I thought we didn't

Re: Review Request 57815: Added `suppressed_roles` field in `SUBSCRIBE`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57815/#review175838 --- Ship it! Ship It! - Vinod Kone On May 19, 2017, 6:26 p.m., A

Re: Review Request 59481: Added sorter test for allocation queries about inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59481/ --- (Updated May 23, 2017, 8:15 p.m.) Review request for mesos, Benjamin Mahler, Ja

Re: Review Request 59428: Moved a couple of logging lines to `INFO` level.

> On May 23, 2017, 8:11 p.m., Vinod Kone wrote: > > Do we typically do `LOG(INFO)` in libprocess? I thought we didn't since > > it's a 3rd party library. The only exceptions I see are in openssl.cpp, > > poll_socket.cpp and profiler.cpp. cc @bmahler As per Till's suggestions, `WARNING` level m

Re: Review Request 59482: Added a new sorter test case, `HierarchicalIterationOrder`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59482/ --- (Updated May 23, 2017, 8:15 p.m.) Review request for mesos, Benjamin Mahler, Ja

Re: Review Request 59481: Added sorter test for allocation queries about inactive clients.

> On May 23, 2017, 3:33 p.m., James Peach wrote: > > I would find it helpful to have a summary of what this test is trying to > > do. I gather that it is demonstrating that even when a client is marked > > inactive, it's share of resources is persisted? Thanks, I added some comments. - Neil

Review Request 59500: Added Windows ReviewBot launch script.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59500/ --- Review request for mesos and Joseph Wu. Repository: mesos Description ---

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59428/ --- (Updated May 23, 2017, 8:20 p.m.) Review request for mesos, Benjamin Mahler, Gr

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

> On May 23, 2017, 8:11 p.m., Vinod Kone wrote: > > Do we typically do `LOG(INFO)` in libprocess? I thought we didn't since > > it's a 3rd party library. The only exceptions I see are in openssl.cpp, > > poll_socket.cpp and profiler.cpp. cc @bmahler > > Anand Mazumdar wrote: > As per Till'

Re: Review Request 59355: Optimized sorter performance with many inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59355/#review175843 --- Patch looks great! Reviews applied: [59481, 59482, 59383, 59483,

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59428/#review175845 --- Ship it! I meant `LOG()` in general. But looks like we do `LOG(

Re: Review Request 53666: Refactor cgroups cleanup to allow cleanup to continue on an error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53666/ --- (Updated May 23, 2017, 8:59 p.m.) Review request for mesos, Jie Yu and Jiang Ya

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58720/#review175849 --- Patch looks great! Reviews applied: [58719, 58720] Passed comman

Re: Review Request 57817: Offers not sent for suppressed roles as indicated in `SUBSCRIBE`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57817/#review175846 --- src/master/allocator/mesos/hierarchical.hpp Line 309 (original),

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57818/#review175850 --- Fix it, then Ship it! src/tests/scheduler_tests.cpp Lines 1490

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59428/#review175856 --- Logging from libprocess seems ok at this point, although ideally t

Re: Review Request 56895: Allow agents to recover slave state post a reboot.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56895/#review175852 --- No tests!? src/slave/slave.cpp Line 5954 (original), 5965 (patch

Re: Review Request 49571: Added a benchmark test for allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49571/#review175857 --- Patch looks great! Reviews applied: [53096, 45962, 55359, 49571]

Re: Review Request 59500: Added Windows ReviewBot launch script.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59500/#review175858 --- Bad patch! Reviews applied: [59500] Failed command: python suppo

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

> On May 23, 2017, 9:31 p.m., Benjamin Mahler wrote: > > Logging from libprocess seems ok at this point, although ideally the user > > of libprocess could configure the logging independently from their own code. > > > > I think the old strategy was, roughly, to use 1 for error, 2 for warning >

Review Request 59504: Removed an instance of double logging in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59504/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description -

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59428/ --- (Updated May 23, 2017, 9:57 p.m.) Review request for mesos, Benjamin Mahler, Gr

Re: Review Request 59428: Moved a couple of logging lines to `WARNING` level.

> On May 23, 2017, 9:31 p.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp > > Lines 3787-3788 (original), 3787-3789 (patched) > > > > > > Is this being double logged? > > > > > > https:/

Re: Review Request 53666: Refactor cgroups cleanup to allow cleanup to continue on an error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53666/#review175869 --- Patch looks great! Reviews applied: [53666] Passed command: supp

Re: Review Request 57817: Offers not sent for suppressed roles as indicated in `SUBSCRIBE`.

> On May 23, 2017, 9:01 p.m., Vinod Kone wrote: > > src/master/allocator/mesos/hierarchical.hpp > > Line 309 (original), 309 (patched) > > > > > > I don't quite follow why you need to have this variable here? Looks

Re: Review Request 57817: Offers not sent for suppressed roles as indicated in `SUBSCRIBE`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57817/ --- (Updated May 23, 2017, 10:19 p.m.) Review request for mesos, James Peach, Vinod

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57818/ --- (Updated May 23, 2017, 10:20 p.m.) Review request for mesos, James Peach, Vinod

Review Request 59506: Consistently use EXIT_ERROR in agent startup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59506/ --- Review request for mesos, Jie Yu and Jiang Yan Xu. Bugs: MESOS-7549 https:/

Re: Review Request 59464: Add Fetcher total and success metrics.

> On May 23, 2017, 1:04 a.m., Joseph Wu wrote: > > What do you think about making fetch failures a sub-metric of > > `container_launch_errors`? The fetcher is only called during container > > launch. If fetching fails, container launch will also fail (and increment > > this counter). It mig

Re: Review Request 59454: Adjust tests to account for GCC 7.1 fix in bytes.hpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59454/#review175875 --- src/tests/role_tests.cpp Line 909 (original), 909 (patched)

Re: Review Request 59434: Added a few master log lines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59434/#review175876 --- Ship it! Ship It! - James Peach On May 22, 2017, 5:38 a.m.,

Re: Review Request 56895: Allow agents to recover slave state post a reboot.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56895/#review175877 --- Patch looks great! Reviews applied: [56895] Passed command: supp

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53840/#review175817 --- Ship it! I'll commit with these adjustments. docs/monitoring.

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59463/ --- (Updated May 23, 2017, 11:59 p.m.) Review request for mesos, Anand Mazumdar, Be

Re: Review Request 59506: Consistently use EXIT() in agent startup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59506/#review175886 --- Fix it, then Ship it! src/slave/main.cpp Line 287 (original),

Re: Review Request 59506: Consistently use EXIT() in agent startup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59506/ --- (Updated May 24, 2017, 12:23 a.m.) Review request for mesos, Jie Yu and Jiang Y

Review Request 59509: Fixed flakiness in MasterAllocatorTest.ResourcesUnused.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59509/ --- Review request for mesos. Bugs: MESOS-7515 https://issues.apache.org/jira/b

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53842/#review175889 --- Bad patch! Reviews applied: [53842, 53841, 53840] Failed command

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59463/#review175888 --- Fix it, then Ship it! src/tests/slave_recovery_tests.cpp Lines

Re: Review Request 53841: Added metrics for sorting in the role and quota sorters.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53841/ --- (Updated May 24, 2017, 4:23 a.m.) Review request for mesos, James Peach and Jia

Re: Review Request 53841: Added metrics for sorting in the role and quota sorters.

> On April 20, 2017, 9:15 p.m., James Peach wrote: > > This also needs documentation added to the `docs/monitoring.md`. Documentation added. - Anindya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apach

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

> On April 25, 2017, 5:31 p.m., James Peach wrote: > > The code looks fine. As per the other revires this needs documentation. > > > > Please update the metric names in the commit. The metrics are > > `allocator/mesos/frameworks/` not > > `allocator/mesos/frameworks/role/`. Added documentatio

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53842/ --- (Updated May 24, 2017, 4:26 a.m.) Review request for mesos, James Peach and Jia

Re: Review Request 59463: Added test for agent ping timeout during agent recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59463/ --- (Updated May 24, 2017, 4:30 a.m.) Review request for mesos, Anand Mazumdar, Ben

Re: Review Request 59481: Added sorter test for allocation queries about inactive clients.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59481/#review175900 --- Ship it! Ship It! - Michael Park On May 23, 2017, 1:15 p.m.,

Re: Review Request 59482: Added a new sorter test case, `HierarchicalIterationOrder`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59482/#review175901 --- Ship it! Ship It! - Michael Park On May 23, 2017, 1:15 p.m.,

Re: Review Request 59500: Added Windows ReviewBot launch script.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59500/#review175902 --- Patch looks great! Reviews applied: [59500] Passed command: expo

Re: Review Request 59383: Added benchmark for allocator perf with many suppressed frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59383/#review175903 --- Fix it, then Ship it! src/tests/hierarchical_allocator_tests.c