Re: Review Request 71310: Stripped metadata and non-scalars from GET_ROLES resources.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71310/#review217318
---



Bad patch!

Reviews applied: [71310]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh 2>&1 | tee 
build_71310"]

Error:
..
77e-a02c-afaff91ed0de@172.17.0.2:33197
I0820 04:45:17.279532 18754 sched.cpp:934] Scheduler::resourceOffers took 
70277ns
I0820 04:45:17.305970 18743 status_update_manager_process.hpp:152] Received 
operation status update OPERATION_FINISHED (Status UUID: 
3c0f90a1-2a81-4fac-896b-8f51ae58351d) for operation UUID 
da4c92ec-359c-4162-8a28-c72d689e8786 on agent 
6ffb108b-662a-4635-aef0-cf5e765eabbd-S0
I0820 04:45:17.306026 18743 status_update_manager_process.hpp:414] Creating 
operation status update stream da4c92ec-359c-4162-8a28-c72d689e8786 
checkpoint=true
I0820 04:45:17.306237 18743 status_update_manager_process.hpp:929] 
Checkpointing UPDATE for operation status update OPERATION_FINISHED (Status 
UUID: 3c0f90a1-2a81-4fac-896b-8f51ae58351d) for operation UUID 
da4c92ec-359c-4162-8a28-c72d689e8786 on agent 
6ffb108b-662a-4635-aef0-cf5e765eabbd-S0
I0820 04:45:17.467989 18743 status_update_manager_process.hpp:528] Forwarding 
operation status update OPERATION_FINISHED (Status UUID: 
3c0f90a1-2a81-4fac-896b-8f51ae58351d) for operation UUID 
da4c92ec-359c-4162-8a28-c72d689e8786 on agent 
6ffb108b-662a-4635-aef0-cf5e765eabbd-S0
I0820 04:45:17.468767 18751 http_connection.hpp:131] Sending 2 call to 
http://172.17.0.2:33197/slave(1224)/api/v1/resource_provider
I0820 04:45:17.469637 18749 process.cpp:3671] Handling HTTP event for process 
'slave(1224)' with path: '/slave(1224)/api/v1/resource_provider'
I0820 04:45:17.472800 18742 process.cpp:3671] Handling HTTP event for process 
'master' with path: '/master/api/v1'
I0820 04:45:17.475044 18754 http.cpp:1115] HTTP POST for /master/api/v1 from 
172.17.0.2:35068
I0820 04:45:17.475404 18754 http.cpp:263] Processing call UNRESERVE_RESOURCES
I0820 04:45:17.476380 18754 master.cpp:3890] Authorizing principal 
'test-principal' to unreserve resources 
'[{"disk":{"source":{"id":"/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_6POiFw/2GB-1e591f03-3726-43fd-b49e-87f0a5881e2e","mount":{"root":"./csi/org.apache.mesos.csi.test/local/mounts"},"profile":"test","type":"MOUNT","vendor":"org.apache.mesos.csi.test.local"}},"name":"disk","provider_id":{"value":"39bd6c79-a7fa-4227-84ab-184a0d1b1858"},"reservations":[{"role":"storage","type":"DYNAMIC"},{"principal":"test-principal","role":"storage/default-role","type":"DYNAMIC"}],"scalar":{"value":2048.0},"type":"SCALAR"}]'
I0820 04:45:17.478766 18752 master.cpp:12724] Removing offer 
6ffb108b-662a-4635-aef0-cf5e765eabbd-O5
I0820 04:45:17.478943 18750 sched.cpp:960] Rescinded offer 
6ffb108b-662a-4635-aef0-cf5e765eabbd-O5
I0820 04:45:17.479032 18750 sched.cpp:971] Scheduler::offerRescinded took 
25387ns
I0820 04:45:17.479735 18748 hierarchical.cpp:1454] Recovered disk(allocated: 
storage/default-role)(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_6POiFw/2GB-1e591f03-3726-43fd-b49e-87f0a5881e2e,test)]:2048;
 cpus(allocated: storage/default-role):2; mem(allocated: 
storage/default-role):1024; disk(allocated: storage/default-role):1024; 
ports(allocated: storage/default-role):[31000-32000] (total: cpus:2; mem:1024; 
disk:1024; ports:[31000-32000]; disk(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_6POiFw/2GB-1e591f03-3726-43fd-b49e-87f0a5881e2e,test)]:2048,
 allocated: {}) on agent 6ffb108b-662a-4635-aef0-cf5e765eabbd-S0 from framework 
6ffb108b-662a-46
 35-aef0-cf5e765eabbd-
I0820 04:45:17.479861 18748 hierarchical.cpp:1500] Framework 
6ffb108b-662a-4635-aef0-cf5e765eabbd- filtered agent 
6ffb108b-662a-4635-aef0-cf5e765eabbd-S0 for 5secs
I0820 04:45:17.482722 18749 master.cpp:12615] Sending operation '' (uuid: 
d0af78e9-ea44-4987-a829-09617b2b33a2) to agent 
6ffb108b-662a-4635-aef0-cf5e765eabbd-S0 at slave(1224)@172.17.0.2:33197 
(6db23a00e664)
I0820 04:45:17.483290 18742 slave.cpp:4352] Ignoring new checkpointed resources 
and operations identical to the current version
I0820 04:45:17.486078 18752 provider.cpp:481] Received APPLY_OPERATION event
I0820 04:45:17.486126 18752 provider.cpp:1295] Received UNRESERVE operation '' 
(uuid: d0af78e9-ea44-4987-a829-09617b2b33a2)

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71151/#review217315
---


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Lines 777 (patched)


Shouldn't `alwaysUpdate` be false here?



src/resource_provider/storage/provider.cpp
Line 753 (original), 820 (patched)


`resourceVersion` needs to be updated every time `UPDATE_STATE` is sent to 
avoid races. In the original implementation, since it is initialized to a 
random UUID in the constructor, we avoided assigning a new random value for the 
first `UPDATE_STATE` call.

In this new codepath, it's possible that we update the random UUID for the 
first `UPDATE_STATE` call, which is okay, just that the initial random UUID is 
wasted. If we don't care about this small overhead, I'd suggest that we move 
this line into the `sendResourceProviderStateUpdate` function.



src/resource_provider/storage/provider.cpp
Lines 760-775 (original), 827-842 (patched)


These should be be removed. L741-753 already handled the transition.



src/resource_provider/storage/provider.cpp
Line 779 (original), 846 (patched)


This should be removed. L739 resumed the SUM.


- Chun-Hung Hsiao


On Aug. 19, 2019, 11:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71151/
> ---
> 
> (Updated Aug. 19, 2019, 11:58 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed periodic storage local provider reconciliations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71151/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Chun-Hung Hsiao]

> On Aug. 9, 2019, 7:41 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/storage/provider.cpp
> > Line 752 (original), 811 (patched)
> > 
> >
> > As commented in the previous patch, since we're having two functions, 
> > `reconcileStoragePools` and `reconcileResources`, it's alright to have a 
> > little bit of repeated code and don't introduce an extra function without a 
> > very focused purpose.
> > 
> > In `reconcileResources`, we could have the logic that checks for 
> > `alwaysUpdate`. In `reconcileStoragePools`, since it's only called when 
> > profiles are updated or volumes of a stale profile are deleted, there's no 
> > need for the extra `alwaysUpdate` logic there.
> 
> Benjamin Bannier wrote:
> I updated the code in question. I am still not sure I understood what 
> exactly you were after, could you have a look?

Since you went for merging `reconfileStoragePools` into `reconcileResources`, 
please ignore it :)


- Chun-Hung


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71151/#review217143
---


On Aug. 19, 2019, 11:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71151/
> ---
> 
> (Updated Aug. 19, 2019, 11:58 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed periodic storage local provider reconciliations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71151/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71150: Factored out storage provider method to update resources.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71150/#review217316
---




src/resource_provider/storage/provider.cpp
Lines 722-723 (original), 729-739 (patched)


Do you think this extra flattern code makes the next continuation more 
readable? If not maybe let's just keep the original `foreach` loop to save an 
extra copy and an extra continuation?

Otherwise, please feel free to drop this. I already gave you another 
ship-it :)


- Chun-Hung Hsiao


On Aug. 19, 2019, 11:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71150/
> ---
> 
> (Updated Aug. 19, 2019, 11:58 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Factored out storage provider method to update resources.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71150/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71150: Factored out storage provider method to update resources.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71150/#review217314
---


Ship it!




Ship It!

- Chun-Hung Hsiao


On Aug. 19, 2019, 11:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71150/
> ---
> 
> (Updated Aug. 19, 2019, 11:58 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Factored out storage provider method to update resources.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71150/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71137: Added a test to ensure composing containerizer preserves request order.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71137/#review217311
---



Patch looks great!

Reviews applied: [71289, 71137]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 14, 2019, 4:12 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71137/
> ---
> 
> (Updated Aug. 14, 2019, 4:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-9887
> https://issues.apache.org/jira/browse/MESOS-9887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure composing containerizer preserves request order.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> 4236bd4ea97f5d2cf91601de34b89e63c85a6fa9 
>   src/tests/slave_tests.cpp 02b65a925a52d09b0a9183d3288b9ab363b98bc5 
> 
> 
> Diff: https://reviews.apache.org/r/71137/diff/2/
> 
> 
> Testing
> ---
> 
> 1. manual testing described in MESOS-9887
> 2. internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71301: Added a framework id field to the allocator Framework struct.

[Meng Zhu]

> On Aug. 19, 2019, 7 a.m., Andrei Sekretenko wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 83 (patched)
> > 
> >
> > What is wrong with `FrameworkInfo.id()`? Do we really need a separate 
> > parameter for an id?

Removed the parameter and added a check ensure `FrameworkInfo` from master has 
id set.


- Meng


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71301/#review217290
---


On Aug. 16, 2019, 3:53 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71301/
> ---
> 
> (Updated Aug. 16, 2019, 3:53 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This provides easy lookup.
> 
> Also refactored related functions to take in `Framework&`
> instead of `FrameworkId&`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 035fd3d7892d4280e299a29f99e8cf6a4c2afb30 
>   src/master/allocator/mesos/hierarchical.cpp 
> b8b9241eae20691ed52c9b5759b17f88a0d8931f 
> 
> 
> Diff: https://reviews.apache.org/r/71301/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71303: Tracked frameworks in the role sorter.

[Meng Zhu]

> On Aug. 19, 2019, 9:44 a.m., Benjamin Mahler wrote:
> > What's the thinking on taking the allocator's framework struct? Why not 
> > just take the information we need so that we can keep the interface 
> > self-contained?

One of the goals of this refactoring is to eliminate state duplication between 
allocator and the sorter. My plan is to let sorter have back pointers pointing 
to both the framework (and the role in subsequent patches) and the role struct 
in the allocator.

Strictly speaking, there is not much information we need about the framework in 
the sorter:
- For random sorter, we only need frameworkId
- For DRF sorter, we only need to know allocations (which can be built in the 
sorter->allocated callbacks, however, note the duplication with the allocator).

Also, ideally, for any potential other sorter implementation that requires any 
other framework info (just a random example: sorting based on capability), 
carry a pointer makes more sense.

I am not sure what's the benefit of keeping the interface self-contained. If 
for unit testing, we can probably still make it unit-testable even if we carry 
the back pointer.


- Meng


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217293
---


On Aug. 16, 2019, 5 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> ---
> 
> (Updated Aug. 16, 2019, 5 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 
> 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 
> 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 
> 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71313: Added a test for RoleTree for basic add and remove operations.

[Meng Zhu]

> On Aug. 19, 2019, 4:12 p.m., Benjamin Mahler wrote:
> > Thanks!
> > 
> > We could probably unit test the `empty()` function (for each case), then 
> > for role tracking, we could use one case (e.g. weights) to test more cases. 
> > Specifically, right now we are only testing the case wehre removal walks 
> > all the way up to the root, but we're missing the case where it has to stop 
> > because an ancestor is !empty.

That would be somewhat whitebox testing. Will add more tests shortly.


- Meng


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71313/#review217306
---


On Aug. 19, 2019, 3:34 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71313/
> ---
> 
> (Updated Aug. 19, 2019, 3:34 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for RoleTree for basic add and remove operations.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 
> 
> 
> Diff: https://reviews.apache.org/r/71313/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71313: Added a test for RoleTree for basic add and remove operations.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71313/#review217306
---


Fix it, then Ship it!




Thanks!

We could probably unit test the `empty()` function (for each case), then for 
role tracking, we could use one case (e.g. weights) to test more cases. 
Specifically, right now we are only testing the case wehre removal walks all 
the way up to the root, but we're missing the case where it has to stop because 
an ancestor is !empty.


src/tests/sorter_tests.cpp
Lines 2189 (patched)


brace on newline

s/TrackingRoles/RolesTracking/



src/tests/sorter_tests.cpp
Lines 2219-2221 (patched)


EXPECT_EQ takes expected first and actual second (and will print out 
accordingly), ditto on flipping all the others in this test


- Benjamin Mahler


On Aug. 19, 2019, 10:34 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71313/
> ---
> 
> (Updated Aug. 19, 2019, 10:34 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for RoleTree for basic add and remove operations.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 
> 
> 
> Diff: https://reviews.apache.org/r/71313/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71285: Fixed recovery of agent resources and operations after crash.

[Greg Mann]

> On Aug. 15, 2019, 5:14 a.m., James Peach wrote:
> > src/slave/state.cpp
> > Lines 819 (patched)
> > 
> >
> > `state::read` can already return `None()`, so what do you think about 
> > letting it return `None()` when the file to read isn't present?
> > 
> > I think that this would improve the code in a number of places, but 
> > it's a large enough change that a separate review would be better.

After looking at the callsites, I'm not convinced that returning `None()` when 
the file doesn't exist will make them simpler. We'll still need checks for 
`result.isNone()`, and I think the code is actually a bit more readable to have 
explicit checks for `os::exists()` instead, rather than hiding this information 
behind the interface of `state::read()`. Let me know what you think, maybe I'm 
missing something.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71285/#review217215
---


On Aug. 19, 2019, 10:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71285/
> ---
> 
> (Updated Aug. 19, 2019, 10:54 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-9875
> https://issues.apache.org/jira/browse/MESOS-9875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed recovery of agent resources and operations after crash.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp e077587fd02bd8e35fee7ce12ae436e3dca25e47 
>   src/slave/paths.cpp 28a7cf9f9c70fb31eeefe2e823cd7e19ffcf126a 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
>   src/slave/state.cpp cd3fac72dd57da21ed5ac46b17066531af26d42a 
> 
> 
> Diff: https://reviews.apache.org/r/71285/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually by doing the following:
> 
> 1) Run a master
> 2) Run an agent with statically reserved resources for use in the following 
> step
> 3) Run the persistent volume example framework to create a volume on the 
> agent, which leads to checkpointing of resources
> 4) Use `chattr -R +i /agent/volumes/dir` to make the agent's persistent 
> volume directory immutable
> 5) Run the persistent volume example framework again; it will fail and the 
> agent will crash
> 6) Restart the agent; confirm that it continues to crash
> 7) Use `chattr -R -i /agent/volumes/dir` to remove the immutable attribute
> 8) Restart the agent; confirm that it recovers successfully, with the 
> persistent volume created on disk
> 9) Run the persistent volume example framework again; it should succeed
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 71285: Fixed recovery of agent resources and operations after crash.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71285/
---

(Updated Aug. 19, 2019, 10:54 p.m.)


Review request for mesos, Gastón Kleiman, James Peach, and Joseph Wu.


Bugs: MESOS-9875
https://issues.apache.org/jira/browse/MESOS-9875


Repository: mesos


Description (updated)
---

Fixed recovery of agent resources and operations after crash.


Diffs (updated)
-

  src/slave/paths.hpp e077587fd02bd8e35fee7ce12ae436e3dca25e47 
  src/slave/paths.cpp 28a7cf9f9c70fb31eeefe2e823cd7e19ffcf126a 
  src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
  src/slave/state.cpp cd3fac72dd57da21ed5ac46b17066531af26d42a 


Diff: https://reviews.apache.org/r/71285/diff/2/

Changes: https://reviews.apache.org/r/71285/diff/1-2/


Testing
---

Tested manually by doing the following:

1) Run a master
2) Run an agent with statically reserved resources for use in the following step
3) Run the persistent volume example framework to create a volume on the agent, 
which leads to checkpointing of resources
4) Use `chattr -R +i /agent/volumes/dir` to make the agent's persistent volume 
directory immutable
5) Run the persistent volume example framework again; it will fail and the 
agent will crash
6) Restart the agent; confirm that it continues to crash
7) Use `chattr -R -i /agent/volumes/dir` to remove the immutable attribute
8) Restart the agent; confirm that it recovers successfully, with the 
persistent volume created on disk
9) Run the persistent volume example framework again; it should succeed


Thanks,

Greg Mann

Re: Review Request 71313: Added a test for RoleTree for basic add and remove operations.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71313/
---

(Updated Aug. 19, 2019, 3:34 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Used the getters to assert states instead of the JSON dump.


Repository: mesos


Description
---

Added a test for RoleTree for basic add and remove operations.


Diffs (updated)
-

  src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 


Diff: https://reviews.apache.org/r/71313/diff/2/

Changes: https://reviews.apache.org/r/71313/diff/1-2/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71318: Added agent reactivations to the existing agent draining tests.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71318/
---

Review request for mesos, Benno Evers and Greg Mann.


Bugs: MESOS-9892
https://issues.apache.org/jira/browse/MESOS-9892


Repository: mesos


Description
---

This adds an extra step to a couple of the agent draining tests,
which calls REACTIVATE_AGENT at the end.


Diffs
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71318/diff/1/


Testing
---

make check


Thanks,

Joseph Wu

Review Request 71317: Added draining test for momentarily disconnected agents.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71317/
---

Review request for mesos, Benno Evers and Greg Mann.


Bugs: MESOS-9892
https://issues.apache.org/jira/browse/MESOS-9892


Repository: mesos


Description
---

This exercises the agent draining code when the agent is disconnected
from the master at the time of starting draining.  Draining is expected
to proceed once the agent reregisters.


Diffs
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71317/diff/1/


Testing
---

make check


Thanks,

Joseph Wu

Review Request 71316: Added draining tests for empty agents.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71316/
---

Review request for mesos, Benno Evers and Greg Mann.


Bugs: MESOS-9892
https://issues.apache.org/jira/browse/MESOS-9892


Repository: mesos


Description
---

This splits the existing agent draining tests into two variants:
1) where the agent has nothing running, and
2) where the agent has one task running.


Diffs
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71316/diff/1/


Testing
---

make check


Thanks,

Joseph Wu

Review Request 71315: Refactored master draining test setup.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71315/
---

Review request for mesos, Benno Evers and Greg Mann.


Bugs: MESOS-9892
https://issues.apache.org/jira/browse/MESOS-9892


Repository: mesos


Description
---

Tests of this feature will generally require a master, agent, framework,
and a single task to be launched at the beginning of the test.
This moves this common code into the test SetUp.

This also changes the `post(...)` helper to return the http::Response
object instead of parsing it.  The response for DRAIN_AGENT calls
does not return an object, so the tests were not checking the
response before.


Diffs
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71315/diff/1/


Testing
---

make check


Thanks,

Joseph Wu

Review Request 71314: Moved master-side agent draining tests into a separate file.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71314/
---

Review request for mesos, Benno Evers and Greg Mann.


Bugs: MESOS-9892
https://issues.apache.org/jira/browse/MESOS-9892


Repository: mesos


Description
---

The test bodies were not changed, besides renaming the test class.


Diffs
-

  src/Makefile.am 0377c8dc4a0f876f972797556c98d88aaab86589 
  src/tests/CMakeLists.txt 04c552a828c6e262bcb66f552b4e42772e33623c 
  src/tests/api_tests.cpp e202cd330d424efef783d39b74db5f856bd34895 
  src/tests/master_draining_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71314/diff/1/


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 71303: Tracked frameworks in the role sorter.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217301
---



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-19 21:44:50 URL:https://reviews.apache.org/r/71254/diff/raw/ 
[5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> ---
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 
> 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 
> 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 
> 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71311: Added method to dump role tree state for debugging.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71311/#review217300
---


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.hpp
Lines 216-217 (patched)


It would be nice if the signature indicated this has to do with json, 
perhaps something like:

```
void toJson(JSON::ObjectWriter* writer) const;
```

I think the reader will get this immediately without needing to read any 
comment or the implementation.

Returning JSON::Object also works, but is a lot more expensive 
unfortunately.



src/master/allocator/mesos/hierarchical.cpp
Lines 398 (patched)


Not including the "role"?


- Benjamin Mahler


On Aug. 19, 2019, 9:03 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71311/
> ---
> 
> (Updated Aug. 19, 2019, 9:03 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It dumps the role tree state in JSON format. This can potentially
> be added to an allocator debug endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71311/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71313: Added a test for RoleTree for basic add and remove operations.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71313/#review217299
---




src/tests/sorter_tests.cpp
Lines 2188 (patched)


Could we call this RoleTracking and either cover all of the lifecycle cases 
(weight, quota, frameworks, and reservations) or have small TrackingByWeight, 
TrackingByQuota, etc for each one?

It will be a lot less verbose if we avoid the json?

```
  RoleTree roleTree;
  roleTree.updateWeight("a/b/c", 2.0);
  
  EXPECT_SOME(roleTree.get("a"));
  EXPECT_SOME(roleTree.get("a/b"));
  EXPECT_SOME(roleTree.get("a/b/c"));
  
  roleTree.updateWeight("a/b/c", DEFAULT_WEIGHT);

  EXPECT_NONE(roleTree.get("a/b/c"));
  EXPECT_NONE(roleTree.get("a/b"));
  EXPECT_NONE(roleTree.get("a"));
  
  etc
```



src/tests/sorter_tests.cpp
Lines 2202-2205 (patched)


Consider omitting some of these fields and using .contains instead? That 
should be more succinct and we don't care about the other fields for lifecycle 
management?


https://github.com/apache/mesos/blob/1.8.1/3rdparty/stout/include/stout/json.hpp#L289-L311

You can find examples in other tests too


- Benjamin Mahler


On Aug. 19, 2019, 9:08 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71313/
> ---
> 
> (Updated Aug. 19, 2019, 9:08 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for RoleTree for basic add and remove operations.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 
> 
> 
> Diff: https://reviews.apache.org/r/71313/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71312: Made RoleTree class default constructable.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71312/#review217298
---


Ship it!





src/master/allocator/mesos/hierarchical.hpp
Lines 191 (patched)


Consider making the test a friend? Or documenting that this one is only for 
testing?



src/master/allocator/mesos/hierarchical.hpp
Line 237 (original), 239 (patched)


Document that None is for testing only?


- Benjamin Mahler


On Aug. 19, 2019, 9:03 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71312/
> ---
> 
> (Updated Aug. 19, 2019, 9:03 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes the metric handler optional. This is mainly for
> unit testing purpose.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71312/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 71313: Added a test for RoleTree for basic add and remove operations.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71313/
---

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

Added a test for RoleTree for basic add and remove operations.


Diffs
-

  src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 


Diff: https://reviews.apache.org/r/71313/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 71269: Added a role tree class in the allocator.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71269/
---

(Updated Aug. 19, 2019, 2:08 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Bugs: MESOS-9917
https://issues.apache.org/jira/browse/MESOS-9917


Repository: mesos


Description
---

The role concept in Mesos fits into a tree structure naturally.
However, the role state in the allocator are currenstored
in a hashmap. This is less efficient and harder to use and reason.

This patch introduced a `RoleTree` structure in the allocator
and organizes all the roles in to a tree. This should simplify
the code logic and opens further refactor and optimization
opportunities.

In addition, the master code also lacks a proper tree structure
for tracking roles. We should leverage the same role tree code
here to simplify that as well.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
8be8dcee04e8fc5b97f730b2f058d14c81678788 
  src/master/allocator/mesos/hierarchical.cpp 
580d35a3b71c1f7e851fa0504c6b9f037c05c378 


Diff: https://reviews.apache.org/r/71269/diff/8/

Changes: https://reviews.apache.org/r/71269/diff/7-8/


Testing
---

make check

Benchmaring using optimized build shows no noticable performance change.

Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`

## Before:

Added 30 agents in 1.252621ms
Added 30 frameworks in 7.747202ms
Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
Made 36 allocations in 12.791427ms

Added 300 agents in 9.765295ms
Added 300 frameworks in 273.978325ms
Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
Made 350 allocations in 424.603736ms

Added 3000 agents in 79.646516ms
Added 3000 frameworks in 19.17449514secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
Made 3500 allocations in 31.687207066secs

## After:

Added 30 agents in 1.376619ms
Added 30 frameworks in 7.647487ms
Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
Made 36 allocations in 11.638116ms

Added 300 agents in 9.506101ms
Added 300 frameworks in 263.008198ms
Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
Made 350 allocations in 418.962396ms

Added 3000 agents in 81.553527ms
Added 3000 frameworks in 19.201708305secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
Made 3500 allocations in 31.583417136secs


Thanks,

Meng Zhu

Review Request 71312: Made RoleTree class default constructable.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71312/
---

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

This makes the metric handler optional. This is mainly for
unit testing purpose.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
8be8dcee04e8fc5b97f730b2f058d14c81678788 
  src/master/allocator/mesos/hierarchical.cpp 
580d35a3b71c1f7e851fa0504c6b9f037c05c378 


Diff: https://reviews.apache.org/r/71312/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71311: Added method to dump role tree state for debugging.

[Meng Zhu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71311/
---

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

It dumps the role tree state in JSON format. This can potentially
be added to an allocator debug endpoint.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
8be8dcee04e8fc5b97f730b2f058d14c81678788 
  src/master/allocator/mesos/hierarchical.cpp 
580d35a3b71c1f7e851fa0504c6b9f037c05c378 


Diff: https://reviews.apache.org/r/71311/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71310: Stripped metadata and non-scalars from GET_ROLES resources.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71310/
---

Review request for mesos, Andrei Sekretenko and Meng Zhu.


Bugs: MESOS-9758
https://issues.apache.org/jira/browse/MESOS-9758


Repository: mesos


Description
---

This endpoint is meant to expose resource statistics, and only
accidentally exposed the metadata and non-scalar resources. It
does not make sense to expose non-scalar resources in this way.

The `resources` field will be deprecated in favor of a breakdown
between offered, allocated, and reserved resources (similar to
the /roles endpoint).


Diffs
-

  src/master/http.cpp 6400771acd2ad7d8948bad7ff8e1eb7fe4546347 
  src/master/master.hpp 09a70dfa7ae306b4de4c688e3b4b4576b610e351 
  src/master/readonly_handler.cpp b226ae1fb1c4ff8ae2cde7e359d4fb1eb5b1b163 
  src/tests/api_tests.cpp e202cd330d424efef783d39b74db5f856bd34895 


Diff: https://reviews.apache.org/r/71310/diff/1/


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 71309: Added more details to the Bintray release guide.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71309/#review217297
---



Patch looks great!

Reviews applied: [71309]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 19, 2019, 5:03 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71309/
> ---
> 
> (Updated Aug. 19, 2019, 5:03 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joseph Wu, Till Toenshoff, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more details to the Bintray release guide.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 88281d691210307a1a6d6c5a9fd71f1607982694 
> 
> 
> Diff: https://reviews.apache.org/r/71309/diff/1/
> 
> 
> Testing
> ---
> 
> Uploaded 1.8.1 package to bintray according to these instructions.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71289: Fixed out-of-order processing of requests in composing containerizer.

[Andrei Budnik]

> On Авг. 19, 2019, 12:02 п.п., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 662 (patched)
> > 
> >
> > Why do we need to call `wait` here? In this case (i.e., 
> > `finalAcknowledgement` is set and container has been removed from 
> > `containers_`), `wait` will always return `None()` right?

added a duplicate comment which clarifies the reason why we need to call `wait` 
(terminated nested container, e.g., a health check).


> On Авг. 19, 2019, 12:02 п.п., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 631-635 (original), 688-695 (patched)
> > 
> >
> > I understand the reason that here we call underlying containerizer's 
> > `status` method is to ensure the previous requests got processed before the 
> > container is removed from composing containerizer's `containers_` map. 
> > However, I am not sure if we can achieve it actually. Let's say the 
> > previous request is UCR's `usage` method, I think with the code here we can 
> > ensure that before the container is removed from composing containerizer's 
> > `containers_` map, the UCR's `usage` method **returns**, but that does not 
> > mean the `usage` method **finishes** all its jobs, i.e., it may just return 
> > a future and the underlying isolators are still collecting the container's 
> > usage.
> > 
> > So I think it might be still possible that a subsequent request to 
> > composing container gets a `Container not found` failure while the previous 
> > request has not finished its jobs yet.

Yeah, I realised that it's not even a composing c'zer bug. The same problem 
might happen with Mesos c'zer.


- Andrei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71289/#review217272
---


On Авг. 14, 2019, 4:11 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71289/
> ---
> 
> (Updated Авг. 14, 2019, 4:11 п.п.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-9887
> https://issues.apache.org/jira/browse/MESOS-9887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the composing containerizer could return
> "Container not found" failure before all preceding requests
> have been processed by the underlying containerizer. It could
> lead to subtle errors on a client side, which expects that all
> requests are processed strictly in the order they arrived.
> This patch introduces DESTROYED state and forces all requests
> to a DESTROYED container to wait until the underlying containerizer
> finishes processing of requests sent before the container transitioned
> to the DESTROYED state.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> d854794fc4775fb8a05efc233d488a64b9ef620a 
> 
> 
> Diff: https://reviews.apache.org/r/71289/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71151/#review217295
---



Patch looks great!

Reviews applied: [71143, 71144, 71145, 71146, 71147, 71148, 71149, 71150, 71151]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 19, 2019, 4:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71151/
> ---
> 
> (Updated Aug. 19, 2019, 4:58 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed periodic storage local provider reconciliations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71151/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71309: Added more details to the Bintray release guide.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71309/#review217294
---


Ship it!




Ship It!

- Vinod Kone


On Aug. 19, 2019, 5:03 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71309/
> ---
> 
> (Updated Aug. 19, 2019, 5:03 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joseph Wu, Till Toenshoff, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more details to the Bintray release guide.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 88281d691210307a1a6d6c5a9fd71f1607982694 
> 
> 
> Diff: https://reviews.apache.org/r/71309/diff/1/
> 
> 
> Testing
> ---
> 
> Uploaded 1.8.1 package to bintray according to these instructions.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 71309: Added more details to the Bintray release guide.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71309/
---

Review request for mesos, Greg Mann, Joseph Wu, Till Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Added more details to the Bintray release guide.


Diffs
-

  docs/release-guide.md 88281d691210307a1a6d6c5a9fd71f1607982694 


Diff: https://reviews.apache.org/r/71309/diff/1/


Testing
---

Uploaded 1.8.1 package to bintray according to these instructions.


Thanks,

Benno Evers

Re: Review Request 71303: Tracked frameworks in the role sorter.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217293
---



What's the thinking on taking the allocator's framework struct? Why not just 
take the information we need so that we can keep the interface self-contained?

- Benjamin Mahler


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> ---
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 
> 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 
> 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 
> 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71269: Added a role tree class in the allocator.

[Benjamin Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71269/#review217292
---


Fix it, then Ship it!




* I think another bug was introduced somewhere along the diffs? See issue below.
* Can we have some unit tests for this before we ship it? There are enough 
tricky cases around tree invariants, and


src/master/allocator/mesos/hierarchical.hpp
Lines 145 (patched)


RoleTree



src/master/allocator/mesos/hierarchical.hpp
Lines 207-209 (patched)


Wrap these two consistently? E.g.

```
  void trackFramework(
  const FrameworkID& frameworkId, const std::string& role);
  void untrackFramework(
  const FrameworkID& frameworkId, const std::string& role);
```



src/master/allocator/mesos/hierarchical.hpp
Lines 226 (patched)


the "tracking only non-empty" invariant may break?

i.e. somehow refer to the invariant that we only track non-empty roles?



src/master/allocator/mesos/hierarchical.cpp
Lines 286 (patched)


Shouldn't this be `!current->isEmpty()`?


- Benjamin Mahler


On Aug. 18, 2019, 7:09 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> ---
> 
> (Updated Aug. 18, 2019, 7:09 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The role concept in Mesos fits into a tree structure naturally.
> However, the role state in the allocator are currenstored
> in a hashmap. This is less efficient and harder to use and reason.
> 
> This patch introduced a `RoleTree` structure in the allocator
> and organizes all the roles in to a tree. This should simplify
> the code logic and opens further refactor and optimization
> opportunities.
> 
> In addition, the master code also lacks a proper tree structure
> for tracking roles. We should leverage the same role tree code
> here to simplify that as well.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71269/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmaring using optimized build shows no noticable performance change.
> 
> Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`
> 
> ## Before:
> 
> Added 30 agents in 1.252621ms
> Added 30 frameworks in 7.747202ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 12.791427ms
> 
> Added 300 agents in 9.765295ms
> Added 300 frameworks in 273.978325ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 424.603736ms
> 
> Added 3000 agents in 79.646516ms
> Added 3000 frameworks in 19.17449514secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.687207066secs
> 
> ## After:
> 
> Added 30 agents in 1.376619ms
> Added 30 frameworks in 7.647487ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 11.638116ms
> 
> Added 300 agents in 9.506101ms
> Added 300 frameworks in 263.008198ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 418.962396ms
> 
> Added 3000 agents in 81.553527ms
> Added 3000 frameworks in 19.201708305secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.583417136secs
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71297: Fixed a flaky operation reconciliation test.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71297/#review217291
---



Patch looks great!

Reviews applied: [71297]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 19, 2019, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71297/
> ---
> 
> (Updated Aug. 19, 2019, 12:36 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9928
> https://issues.apache.org/jira/browse/MESOS-9928
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The FrameworkReconciliationRaceWithUpdateSlave test from the
> operation reconciliation tests was flaky since we did not wait
> for the scheduler to reconnect before attempting to send a
> subscribe call.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 9d084c027ec2f910515cafebf715f7428c43f1a9 
> 
> 
> Diff: https://reviews.apache.org/r/71297/diff/2/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
> --gtest_repeat=200` while simultaneously running `stress-ng` in the 
> background.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71301: Added a framework id field to the allocator Framework struct.

[Andrei Sekretenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71301/#review217290
---




src/master/allocator/mesos/hierarchical.hpp
Lines 83 (patched)


What is wrong with `FrameworkInfo.id()`? Do we really need a separate 
parameter for an id?


- Andrei Sekretenko


On Aug. 16, 2019, 10:53 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71301/
> ---
> 
> (Updated Aug. 16, 2019, 10:53 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This provides easy lookup.
> 
> Also refactored related functions to take in `Framework&`
> instead of `FrameworkId&`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71301/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71297: Fixed a flaky operation reconciliation test.

[Benno Evers]

> On Aug. 16, 2019, 2:43 p.m., Andrei Sekretenko wrote:
> > src/tests/operation_reconciliation_tests.cpp
> > Lines 1842 (patched)
> > 
> >
> > We are restarting the master once, but the scheduler gets 
> > connected/disconnected event pair twice during the master restart... is 
> > this guaranteed by TestMesos + StandaloneMasterDetector, a stable 
> > coincidence, or just the most likely outcome of some other race? 
> > 
> > IMO, this is at least worth a comment - but what about preventing it? 
> > (Will something like `detector->appoint(None())` + 
> > `AWAIT_READY(disconnected)` before killing the master help?)

Great idea, thanks!


- Benno


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71297/#review217239
---


On Aug. 19, 2019, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71297/
> ---
> 
> (Updated Aug. 19, 2019, 12:36 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9928
> https://issues.apache.org/jira/browse/MESOS-9928
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The FrameworkReconciliationRaceWithUpdateSlave test from the
> operation reconciliation tests was flaky since we did not wait
> for the scheduler to reconnect before attempting to send a
> subscribe call.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 9d084c027ec2f910515cafebf715f7428c43f1a9 
> 
> 
> Diff: https://reviews.apache.org/r/71297/diff/2/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
> --gtest_repeat=200` while simultaneously running `stress-ng` in the 
> background.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71297: Fixed a flaky operation reconciliation test.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71297/
---

(Updated Aug. 19, 2019, 12:36 p.m.)


Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
Toenshoff.


Bugs: MESOS-9928
https://issues.apache.org/jira/browse/MESOS-9928


Repository: mesos


Description (updated)
---

The FrameworkReconciliationRaceWithUpdateSlave test from the
operation reconciliation tests was flaky since we did not wait
for the scheduler to reconnect before attempting to send a
subscribe call.


Diffs (updated)
-

  src/tests/operation_reconciliation_tests.cpp 
9d084c027ec2f910515cafebf715f7428c43f1a9 


Diff: https://reviews.apache.org/r/71297/diff/2/

Changes: https://reviews.apache.org/r/71297/diff/1-2/


Testing
---

`./src/mesos-tests 
--gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
--gtest_repeat=200` while simultaneously running `stress-ng` in the background.


Thanks,

Benno Evers

Re: Review Request 71289: Fixed out-of-order processing of requests in composing containerizer.

[Qian Zhang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71289/#review217272
---




src/slave/containerizer/composing.cpp
Line 136 (original), 137-138 (patched)


A newline between.



src/slave/containerizer/composing.cpp
Lines 325-326 (original), 336-337 (patched)


Better to name the method as `waited` rather than `destroyed`? Since it 
gets called when `wait` is done.



src/slave/containerizer/composing.cpp
Lines 541 (patched)


Kill this blank line, and ditto for other places.



src/slave/containerizer/composing.cpp
Lines 662 (patched)


Why do we need to call `wait` here? In this case (i.e., 
`finalAcknowledgement` is set and container has been removed from 
`containers_`), `wait` will always return `None()` right?



src/slave/containerizer/composing.cpp
Lines 631-635 (original), 688-695 (patched)


I understand the reason that here we call underlying containerizer's 
`status` method is to ensure the previous requests got processed before the 
container is removed from composing containerizer's `containers_` map. However, 
I am not sure if we can achieve it actually. Let's say the previous request is 
UCR's `usage` method, I think with the code here we can ensure that before the 
container is removed from composing containerizer's `containers_` map, the 
UCR's `usage` method **returns**, but that does not mean the `usage` method 
**finishes** all its jobs, i.e., it may just return a future and the underlying 
isolators are still collecting the container's usage.

So I think it might be still possible that a subsequent request to 
composing container gets a `Container not found` failure while the previous 
request has not finished its jobs yet.


- Qian Zhang


On Aug. 15, 2019, 12:11 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71289/
> ---
> 
> (Updated Aug. 15, 2019, 12:11 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-9887
> https://issues.apache.org/jira/browse/MESOS-9887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the composing containerizer could return
> "Container not found" failure before all preceding requests
> have been processed by the underlying containerizer. It could
> lead to subtle errors on a client side, which expects that all
> requests are processed strictly in the order they arrived.
> This patch introduces DESTROYED state and forces all requests
> to a DESTROYED container to wait until the underlying containerizer
> finishes processing of requests sent before the container transitioned
> to the DESTROYED state.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> d854794fc4775fb8a05efc233d488a64b9ef620a 
> 
> 
> Diff: https://reviews.apache.org/r/71289/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Benjamin Bannier]

> On Aug. 9, 2019, 9:41 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/storage/provider.cpp
> > Line 752 (original), 811 (patched)
> > 
> >
> > As commented in the previous patch, since we're having two functions, 
> > `reconcileStoragePools` and `reconcileResources`, it's alright to have a 
> > little bit of repeated code and don't introduce an extra function without a 
> > very focused purpose.
> > 
> > In `reconcileResources`, we could have the logic that checks for 
> > `alwaysUpdate`. In `reconcileStoragePools`, since it's only called when 
> > profiles are updated or volumes of a stale profile are deleted, there's no 
> > need for the extra `alwaysUpdate` logic there.

I updated the code in question. I am still not sure I understood what exactly 
you were after, could you have a look?


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71151/#review217143
---


On Aug. 19, 2019, 1:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71151/
> ---
> 
> (Updated Aug. 19, 2019, 1:58 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed periodic storage local provider reconciliations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71151/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71150: Factored out storage provider method to update resources.

[Benjamin Bannier]

> On Aug. 9, 2019, 9:22 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 979-995 (patched)
> > 
> >
> > Sorry for not making my suggestion clear enough. I was actually 
> > thinking about removing `reconcileStoragePools()` and always calling 
> > `reconcileResources()` even when we only want to reconcile storage pools.
> > 
> > This suggestion makes more sense if we don't reconcile storage pools 
> > after destroying a MOUNT disk with a stale profile, as I suggested in the 
> > next patch. But if we want to keep this behavior, then this approach would 
> > introduce an extra `ListVolumes` call.
> > 
> > If you prefer to avoid having this extra grpc call, then adding this 
> > extra function seems only give us a little bit of code reuse, and I'm not 
> > sure if it worths adding an extra function name to the already-long list of 
> > functions in this class. I'd vote for keeping the code in its original 
> > place and avoid introducing a function name that doesn't convey its purpose 
> > clearly.

Updated the code, PTAL.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71150/#review217142
---


On Aug. 19, 2019, 1:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71150/
> ---
> 
> (Updated Aug. 19, 2019, 1:58 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Factored out storage provider method to update resources.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71150/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71151/
---

(Updated Aug. 19, 2019, 1:58 p.m.)


Review request for mesos and Chun-Hung Hsiao.


Changes
---

Address issue raised by Chun


Bugs: MESOS-9254
https://issues.apache.org/jira/browse/MESOS-9254


Repository: mesos


Description
---

Performed periodic storage local provider reconciliations.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 
  src/tests/storage_local_resource_provider_tests.cpp 
69b59d48ceefebbb7accefe411c54ac5cecff1c3 


Diff: https://reviews.apache.org/r/71151/diff/6/

Changes: https://reviews.apache.org/r/71151/diff/5-6/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71150: Factored out storage provider method to update resources.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71150/
---

(Updated Aug. 19, 2019, 1:58 p.m.)


Review request for mesos and Chun-Hung Hsiao.


Changes
---

Address issue raised by Chun


Bugs: MESOS-9254
https://issues.apache.org/jira/browse/MESOS-9254


Repository: mesos


Description
---

Factored out storage provider method to update resources.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 
  src/tests/storage_local_resource_provider_tests.cpp 
69b59d48ceefebbb7accefe411c54ac5cecff1c3 


Diff: https://reviews.apache.org/r/71150/diff/5/

Changes: https://reviews.apache.org/r/71150/diff/4-5/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71208: Revert "Updated cpplint to be compatible with Python 3."

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71208/#review217286
---


Ship it!




Ship It!

- Benno Evers


On July 30, 2019, 9:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71208/
> ---
> 
> (Updated July 30, 2019, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit 89db66e3df831eaa50fffb4149a3894097505c14.
> 
> This patch was necessary when we were running cpplint in the python3
> environment used e.g., also for bindings and other scripts. With
> pre-commit we have freedom to choose the Python environment needed so we
> can undo our adjustments here to stay closer to upstream.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 66ec8b3636a8d3ba57becd8560b4fe394e7119d8 
> 
> 
> Diff: https://reviews.apache.org/r/71208/diff/2/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71207: Revert "Updated cpplint.py to be less verbose when there is no linting issue."

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71207/#review217285
---


Ship it!




Ship It!

- Benno Evers


On July 30, 2019, 9:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71207/
> ---
> 
> (Updated July 30, 2019, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit c0f8f56d5a93f3fb870e448fedfd22f1491356ca.
> 
> This patch was necessary when we were running cpplint via
> `support/mesos-style.py` to prevent it from cluttering up the hook
> output. When running under pre-commit linter output is not shown if no
> errors occur so we can undo our change to stay closer to upstream.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 66ec8b3636a8d3ba57becd8560b4fe394e7119d8 
> 
> 
> Diff: https://reviews.apache.org/r/71207/diff/2/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71205: Switched commit hooks to pre-commit.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217283
---


Ship it!




Ship It!

- Benno Evers


On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> ---
> 
> (Updated Aug. 19, 2019, 7:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
> 
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
> 
> 
> Diffs
> -
> 
>   docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/pre-commit-config.yaml PRE-CREATION 
>   support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/setup-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71205/diff/7/
> 
> 
> Testing
> ---
> 
> * used successfully for a couple of months
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71209: Enabled a number of additional pre-commit checks.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71209/#review217284
---


Ship it!




Ship It!

- Benno Evers


On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71209/
> ---
> 
> (Updated Aug. 19, 2019, 7:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled a number of additional pre-commit checks.
> 
> 
> Diffs
> -
> 
>   support/pre-commit-config.yaml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71209/diff/5/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree as indentified issues were 
> fixed
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70096: Moved cpplint configuration into dedicated file.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70096/#review217282
---


Ship it!




Ship It!

- Benno Evers


On Aug. 14, 2019, 11:25 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70096/
> ---
> 
> (Updated Aug. 14, 2019, 11:25 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this change we not only reduce the amount of code in
> `support/mesos-style.py` in favor of a configuration supported by
> upstream, but we also make it easier to interoperate with editor
> integrations for cpplint.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/CPPLINT.cfg PRE-CREATION 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/70096/diff/7/
> 
> 
> Testing
> ---
> 
> * confirmed that `./support/mesos-style.py src/executor/executor.cpp` still 
> does what is expected
> * no new warnings when running over the whole codebase
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71203: Added check script to check for license headers.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71203/#review217281
---


Ship it!




Ship It!

- Benno Evers


On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71203/
> ---
> 
> (Updated Aug. 19, 2019, 7:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check adds a script which validates that source files have valid
> license headers. This will allow us to reuse this functionality with
> e.g., the pre-commit tool.
> 
> At the moment the code added here is not invoked from
> `support/mesos-style.py` since it will be removed in a follow-up commit.
> 
> 
> Diffs
> -
> 
>   support/check-license.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71203/diff/5/
> 
> 
> Testing
> ---
> 
> * tested against files with license headers present or absent
> * tested against all Python and C++ source files in the repo
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71204: Added gitlint config.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71204/#review217280
---


Ship it!




Ship It!

- Benno Evers


On Aug. 14, 2019, 11:24 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71204/
> ---
> 
> (Updated Aug. 14, 2019, 11:24 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a config for the gitlint tool which is slated to replace
> a custom commit-msg hook once we switch our hook infrastructure to the
> pre-commit tool.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/gitlint PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71204/diff/7/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71206: Removed old mesos-style and references.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71206/#review217279
---


Fix it, then Ship it!





docs/c++-style-guide.md
Line 10 (original), 10 (patched)


s/script/command/



support/mesos-style.py
Line 28 (original), 25 (patched)


This is probably going to be the first point of contact with the new 
linting system for most developers, so it might pay to be a bit more verbose, 
e.g. mention that you can install `pre-commit` via pip and add links to the 
pre-commit website, mailing list or JIRA tickets, etc.

However, it's essentially a judgment call so I'll leave it up to you :D


- Benno Evers


On Aug. 16, 2019, 7:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71206/
> ---
> 
> (Updated Aug. 16, 2019, 7:42 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch removes references to `support/mesos-style.py` which was
> replaced with a pre-commit setup in a previous commit. We also remove
> the tool itself.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 8a48afe780f23736c9b7abeb7337977521cecfa5 
>   support/build-virtualenv 7dc03b054f7663979e4eb4b11ad51d759b7f1ad3 
>   support/hooks/commit-msg a0c218deee3fb4b7594fe39b76c1025045ba0725 
>   support/hooks/post-rewrite 1ab14abf711d1923a7ae69beb33581317009a94a 
>   support/hooks/pre-commit 519567bf5f20a74b273c8d8514577fe4342dc45d 
>   support/mesos-split.py 0a77c257386ffe576abd12f59f926640836ad900 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/71206/diff/4/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71299: Added separate script to install developer setup.

[Benno Evers]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71299/#review217278
---


Fix it, then Ship it!





docs/advanced-contribution.md
Line 69 (original), 69 (patched)


This became a bit ambiguous now, i.e. is only the second step or the whole 
step only required if building from git?

(also, I'm not sure why we have this qualification at all in a contributors 
guide)


- Benno Evers


On Aug. 19, 2019, 7:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71299/
> ---
> 
> (Updated Aug. 19, 2019, 7:19 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch breaks the installation of developer tools (i.e., linter
> configuration files and git hooks) out of `./bootstrap`. This not only
> simplifies and streamlines the setup, but will allow us to add
> developer-only features without breaking users who are just interested
> in building a distribution tarball.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
>   docs/beginner-contribution.md 471f5dd0da30d921cc3b29484d2b02f164f8ca75 
>   docs/clang-format.md 4289813c2ca78e1b963aa22a596abd6e8aa3a28b 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/llvm/README.md 188930224ba8ff53dbef8770bc56625be936bb76 
>   support/setup-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71299/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

[GitHub] [mesos] bbannier merged pull request #342: Made `support/cpplint.py` executable.

[GitBox]

bbannier merged pull request #342: Made `support/cpplint.py` executable.
URL: https://github.com/apache/mesos/pull/342
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mesos] bbannier opened a new pull request #342: Made `support/cpplint.py` executable.

[GitBox]

bbannier opened a new pull request #342: Made `support/cpplint.py` executable.
URL: https://github.com/apache/mesos/pull/342
 
 
   This allows invoking cpplint from outside the linter setup so that one
   can make use of our patched cpplint even when not using mesos-style.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: Review Request 71300: Removed mesos-style transition script.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71300/#review217275
---



Patch looks great!

Reviews applied: [70096, 71203, 71204, 71299, 71205, 71206, 71207, 71208, 
71209, 71300]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 16, 2019, 7:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71300/
> ---
> 
> (Updated Aug. 16, 2019, 7:42 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed mesos-style transition script.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/71300/diff/2/
> 
> 
> Testing
> ---
> 
> n/a
> 
> THIS PATCH SHOULD ONLY BE COMMITTED AFTER THE PRECEEDING CHAIN HAS BEEN 
> LANDED FOR SOME TIME TO GIVE CONTRIBUTORS A CHANCE TO ADJUST THEIR WORKFLOW.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71205: Switched commit hooks to pre-commit.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
---

(Updated Aug. 19, 2019, 9:20 a.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase; install config via link to allow for untracked modifications


Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630


Repository: mesos


Description
---

This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).

Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.


Diffs (updated)
-

  docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
  support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
  support/pre-commit-config.yaml PRE-CREATION 
  support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
  support/setup-dev.sh PRE-CREATION 


Diff: https://reviews.apache.org/r/71205/diff/7/

Changes: https://reviews.apache.org/r/71205/diff/6-7/


Testing
---

* used successfully for a couple of months


Thanks,

Benjamin Bannier

Re: Review Request 71209: Enabled a number of additional pre-commit checks.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71209/
---

(Updated Aug. 19, 2019, 9:20 a.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase


Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630


Repository: mesos


Description
---

Enabled a number of additional pre-commit checks.


Diffs (updated)
-

  support/pre-commit-config.yaml PRE-CREATION 


Diff: https://reviews.apache.org/r/71209/diff/4/

Changes: https://reviews.apache.org/r/71209/diff/3-4/


Testing
---

* used for development for a couple of months
* reports no issues in the current source tree as indentified issues were fixed


Thanks,

Benjamin Bannier

Re: Review Request 71203: Added check script to check for license headers.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71203/
---

(Updated Aug. 19, 2019, 9:20 a.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rename script


Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630


Repository: mesos


Description
---

This check adds a script which validates that source files have valid
license headers. This will allow us to reuse this functionality with
e.g., the pre-commit tool.

At the moment the code added here is not invoked from
`support/mesos-style.py` since it will be removed in a follow-up commit.


Diffs (updated)
-

  support/check-license.py PRE-CREATION 


Diff: https://reviews.apache.org/r/71203/diff/5/

Changes: https://reviews.apache.org/r/71203/diff/4-5/


Testing
---

* tested against files with license headers present or absent
* tested against all Python and C++ source files in the repo


Thanks,

Benjamin Bannier

Re: Review Request 71299: Added separate script to install developer setup.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71299/
---

(Updated Aug. 19, 2019, 9:19 a.m.)


Review request for mesos, Benno Evers and Till Toenshoff.


Changes
---

Rename setup script


Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630


Repository: mesos


Description
---

This patch breaks the installation of developer tools (i.e., linter
configuration files and git hooks) out of `./bootstrap`. This not only
simplifies and streamlines the setup, but will allow us to add
developer-only features without breaking users who are just interested
in building a distribution tarball.


Diffs (updated)
-

  bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
  bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
  docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
  docs/beginner-contribution.md 471f5dd0da30d921cc3b29484d2b02f164f8ca75 
  docs/clang-format.md 4289813c2ca78e1b963aa22a596abd6e8aa3a28b 
  support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
  support/llvm/README.md 188930224ba8ff53dbef8770bc56625be936bb76 
  support/setup-dev.sh PRE-CREATION 


Diff: https://reviews.apache.org/r/71299/diff/4/

Changes: https://reviews.apache.org/r/71299/diff/3-4/


Testing
---


Thanks,

Benjamin Bannier