Re: Review Request 71629: Improved logging in Docker store and registry puller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71629/#review218286 --- Fix it, then Ship it! src/slave/containerizer/mesos/provisioner/docker/store.cpp Lines 430 (patched) <https://reviews.apache.org/r/71629/#comment305953> "moving layers from staging dir '' to image store"? - Gilbert Song On Oct. 17, 2019, 5:58 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71629/ > --- > > (Updated Oct. 17, 2019, 5:58 p.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > Improved logging in Docker store and registry puller. > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp > 35b6afbb6b22575b90963927352443a8ddaf9885 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > > > Diff: https://reviews.apache.org/r/71629/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71629: Improved logging in Docker store and registry puller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71629/#review218275 --- src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp Lines 457-458 (original), 456-457 (patched) <https://reviews.apache.org/r/71629/#comment305919> Is this too verbose? Each image may have 100 lines of this log src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp Lines 533-534 (original), 532-533 (patched) <https://reviews.apache.org/r/71629/#comment305920> ditto src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp Lines 544-545 (original), 543-544 (patched) <https://reviews.apache.org/r/71629/#comment305921> ditto src/slave/containerizer/mesos/provisioner/docker/store.cpp Lines 358 (patched) <https://reviews.apache.org/r/71629/#comment305922> we probably want to log this line in metadatamanager? - Gilbert Song On Oct. 17, 2019, 2:45 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71629/ > --- > > (Updated Oct. 17, 2019, 2:45 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > Improved logging in Docker store and registry puller. > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp > 35b6afbb6b22575b90963927352443a8ddaf9885 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > > > Diff: https://reviews.apache.org/r/71629/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71608: Supported destroying UCR container in `PROVISIONING` state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71608/#review218274 --- Ship it! Ship It! - Gilbert Song On Oct. 17, 2019, 12:15 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71608/ > --- > > (Updated Oct. 17, 2019, 12:15 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > Previously in MESOS-3736, we made Docker store support pulling same > image simultaneously which is a performance improvement, however it > may cause an issue: If the pulling hangs somehow, all the subsequent > pulling request for the same image will hang as well, and as a result > the container destroy will also hang since destroy has to wait for > provisioning to finish, see MESOS-4985 for details. > > So in this patch we removed that performance improvement and made UCR > can destroy the container which is being provisioned, i.e., UCR will > discard the container provisioning and then keep doing the container > destroy. And we also improved Docker fetcher plugin so that when > container provisioning is discarded the `curl` process used to fetch > manifest or blob will be killed immediately. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.hpp > 6537f6550b353b1c65a30381f6d68f61508d4960 > src/slave/containerizer/mesos/containerizer.cpp > c61b954635ebcaed3e498452bddeee6e1b34f388 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > src/slave/containerizer/mesos/provisioner/provisioner.hpp > 38664177ae3805041b412ed52dd0134a6f3aa679 > src/slave/containerizer/mesos/provisioner/provisioner.cpp > 3d0b291fa878ab22625435afe2a219d6f776e52c > src/tests/containerizer/mesos_containerizer_tests.cpp > 449928c10b897061642af8ad267f8b70695940e6 > src/tests/containerizer/provisioner_docker_tests.cpp > 5d5a355afd9c4fda1c653d6cecb75703b0fe > src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2 > > > Diff: https://reviews.apache.org/r/71608/diff/5/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71608: Supported destroying UCR container in `PROVISIONING` state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71608/#review218254 --- Ship it! Ship It! - Gilbert Song On Oct. 16, 2019, 6:43 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71608/ > --- > > (Updated Oct. 16, 2019, 6:43 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > Previously in MESOS-3736, we made Docker store support pulling same > image simultaneously which is a performance improvement, however it > may cause an issue: If the pulling hangs somehow, all the subsequent > pulling request for the same image will hang as well, and as a result > the container destroy will also hang since destroy has to wait for > provisioning to finish, see MESOS-4985 for details. > > So in this patch we removed that performance improvement and made UCR > can destroy the container which is being provisioned, i.e., UCR will > discard the container provisioning and then keep doing the container > destroy. And we also improved Docker fetcher plugin so that when > container provisioning is discarded the `curl` process used to fetch > manifest or blob will be killed immediately. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.hpp > 6537f6550b353b1c65a30381f6d68f61508d4960 > src/slave/containerizer/mesos/containerizer.cpp > c61b954635ebcaed3e498452bddeee6e1b34f388 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > src/slave/containerizer/mesos/provisioner/provisioner.hpp > 38664177ae3805041b412ed52dd0134a6f3aa679 > src/slave/containerizer/mesos/provisioner/provisioner.cpp > 3d0b291fa878ab22625435afe2a219d6f776e52c > src/tests/containerizer/mesos_containerizer_tests.cpp > 449928c10b897061642af8ad267f8b70695940e6 > src/tests/containerizer/provisioner_docker_tests.cpp > 5d5a355afd9c4fda1c653d6cecb75703b0fe > src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2 > > > Diff: https://reviews.apache.org/r/71608/diff/4/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71608: WIP: Supported destroying UCR container in `PROVISIONING` state.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71608/#review218206
---
Fix it, then Ship it!
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 296-312 (patched)
<https://reviews.apache.org/r/71608/#comment305780>
could you just do:
```
promise->associate(future);
return promise->future()
.onDiscard([promise]() {
promise->discard();
});
```
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 189 (patched)
<https://reviews.apache.org/r/71608/#comment305779>
smart move! we decouple the destroy from image pulling to avoid a write
lock!
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Line 521 (original), 522 (patched)
<https://reviews.apache.org/r/71608/#comment305778>
Just double checking: once we discard the store::get() above, this .then
will not continue?
- Gilbert Song
On Oct. 14, 2019, 7:50 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71608/
> ---
>
> (Updated Oct. 14, 2019, 7:50 a.m.)
>
>
> Review request for mesos and Gilbert Song.
>
>
> Bugs: MESOS-9964
> https://issues.apache.org/jira/browse/MESOS-9964
>
>
> Repository: mesos
>
>
> Description
> ---
>
> WIP: Supported destroying UCR container in `PROVISIONING` state.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/containerizer.hpp
> 6537f6550b353b1c65a30381f6d68f61508d4960
> src/slave/containerizer/mesos/containerizer.cpp
> c61b954635ebcaed3e498452bddeee6e1b34f388
> src/slave/containerizer/mesos/provisioner/docker/store.cpp
> 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2
> src/slave/containerizer/mesos/provisioner/provisioner.hpp
> 38664177ae3805041b412ed52dd0134a6f3aa679
> src/slave/containerizer/mesos/provisioner/provisioner.cpp
> 3d0b291fa878ab22625435afe2a219d6f776e52c
> src/tests/containerizer/provisioner_docker_tests.cpp
> 5d5a355afd9c4fda1c653d6cecb75703b0fe
> src/uri/fetchers/docker.cpp 8f5fc964f056b349ce57ced139e07f538cb1cfd2
>
>
> Diff: https://reviews.apache.org/r/71608/diff/2/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71608: WIP: Supported destroying UCR container in `PROVISIONING` state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71608/#review218194 --- src/slave/containerizer/mesos/provisioner/docker/store.cpp Line 354 (original), 348 (patched) <https://reviews.apache.org/r/71608/#comment305749> we dont need this promise in this case. src/slave/containerizer/mesos/provisioner/docker/store.cpp Lines 379 (patched) <https://reviews.apache.org/r/71608/#comment305750> call future.discard() if we remove this promise - Gilbert Song On Oct. 11, 2019, 7:09 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71608/ > --- > > (Updated Oct. 11, 2019, 7:09 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > WIP: Supported destroying UCR container in `PROVISIONING` state. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > c61b954635ebcaed3e498452bddeee6e1b34f388 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > > > Diff: https://reviews.apache.org/r/71608/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71608: WIP: Supported destroying UCR container in `PROVISIONING` state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71608/#review218193 --- we need to remove this test `PullingSameImageSimutanuously` - Gilbert Song On Oct. 11, 2019, 7:09 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71608/ > --- > > (Updated Oct. 11, 2019, 7:09 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9964 > https://issues.apache.org/jira/browse/MESOS-9964 > > > Repository: mesos > > > Description > --- > > WIP: Supported destroying UCR container in `PROVISIONING` state. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > c61b954635ebcaed3e498452bddeee6e1b34f388 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 286ee5f00933b4c86a96dc4e10e42f9e7eac0ce2 > > > Diff: https://reviews.apache.org/r/71608/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71518: Added the test `GarbageCollectorIntegrationTest.ROOT_OrphanContainer`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71518/#review217971 --- Ship it! Ship It! - Gilbert Song On Sept. 19, 2019, 8:06 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71518/ > --- > > (Updated Sept. 19, 2019, 8:06 p.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9966 > https://issues.apache.org/jira/browse/MESOS-9966 > > > Repository: mesos > > > Description > --- > > Added the test `GarbageCollectorIntegrationTest.ROOT_OrphanContainer`. > > > Diffs > - > > src/tests/gc_tests.cpp 2ea4bcb668e1fcbeb9c598053e4df4d54d17711d > > > Diff: https://reviews.apache.org/r/71518/diff/2/ > > > Testing > --- > > sudo make check > > This test will fail without the previous patch r/71501. > > > Thanks, > > Qian Zhang > >
Re: Review Request 71501: Gc'ed nested container sandbox only if we have root container sandbox.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71501/#review217970 --- Ship it! I remember we used to fix an similar issue - root container sandbox is not recovered. could you remind me how could we get into this case? - Gilbert Song On Sept. 18, 2019, 3:49 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71501/ > --- > > (Updated Sept. 18, 2019, 3:49 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9966 > https://issues.apache.org/jira/browse/MESOS-9966 > > > Repository: mesos > > > Description > --- > > Gc'ed nested container sandbox only if we have root container sandbox. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > 16149a1428db687b131de08d14893a2dd684ce28 > > > Diff: https://reviews.apache.org/r/71501/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71454: Added `futureTracker` to the `SlaveOptions` in tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71454/#review217916 --- Ship it! Ship It! - Gilbert Song On Sept. 9, 2019, 7:53 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71454/ > --- > > (Updated Sept. 9, 2019, 7:53 a.m.) > > > Review request for mesos, Gilbert Song, Joseph Wu, and Qian Zhang. > > > Bugs: MESOS-9843 > https://issues.apache.org/jira/browse/MESOS-9843 > > > Repository: mesos > > > Description > --- > > `PendingFutureTracker` is shared across both Mesos containerizer and > the agent, so we need to add an option to be able to start a slave in > tests with an instance of the `futureTrack` as a parameter. > > > Diffs > - > > src/tests/cluster.hpp c04ee14beafe350568a295c8258566aa3704028a > src/tests/cluster.cpp 1646516f1bacc2371eb420f118b656c8c475367c > src/tests/mesos.hpp ecde51828cb74b34d8ee338259e3ece3944a17f1 > src/tests/mesos.cpp e77db2231cd81a272c2b7143a48ee193e63e54ab > > > Diff: https://reviews.apache.org/r/71454/diff/2/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71509: Windows: Fixed AllocationRoleEnvironmentVariable tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71509/#review217837 --- Ship it! Ship It! - Gilbert Song On Sept. 18, 2019, 3:57 p.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71509/ > --- > > (Updated Sept. 18, 2019, 3:57 p.m.) > > > Review request for mesos, Gilbert Song, Greg Mann, Qian Zhang, and Till > Toenshoff. > > > Bugs: MESOS-9874 > https://issues.apache.org/jira/browse/MESOS-9874 > > > Repository: mesos > > > Description > --- > > These tests were using a shell script, which can easily be > converted to a batch script on Windows. > > > Diffs > - > > src/tests/command_executor_tests.cpp > e9726b0b6c4623d5196186cd43722952f77669da > src/tests/containerizer/docker_containerizer_tests.cpp > 5e31a95149bdc0de64eeab3c289f7816077c82c4 > src/tests/default_executor_tests.cpp > e5c0bf2df4b35fb3920104e1b72bea7797724933 > > > Diff: https://reviews.apache.org/r/71509/diff/1/ > > > Testing > --- > > cmake --build . --target check (Windows) > > > Thanks, > > Joseph Wu > >
Re: Review Request 71378: Added node draining to 1.9.0 CHANGELOG.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71378/#review217472 --- Ship it! Ship It! - Gilbert Song On Aug. 27, 2019, 6:19 p.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71378/ > --- > > (Updated Aug. 27, 2019, 6:19 p.m.) > > > Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang. > > > Repository: mesos > > > Description > --- > > Added node draining to 1.9.0 CHANGELOG. > > > Diffs > - > > CHANGELOG 1c631978385925433e12f8854c47b6f4056cb577 > > > Diff: https://reviews.apache.org/r/71378/diff/1/ > > > Testing > --- > > > Thanks, > > Joseph Wu > >
Re: Review Request 71375: Added quota limits to the 1.9.0 CHANGELOG.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71375/#review217456 --- Ship it! Ship It! - Gilbert Song On Aug. 27, 2019, 9:56 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71375/ > --- > > (Updated Aug. 27, 2019, 9:56 a.m.) > > > Review request for mesos, Andrei Sekretenko, Meng Zhu, Qian Zhang, and Vinod > Kone. > > > Repository: mesos > > > Description > --- > > Added quota limits to the 1.9.0 CHANGELOG. > > > Diffs > - > > CHANGELOG 8ce9ee6a5f55b37765d53c5735d20c74be5ce8e2 > > > Diff: https://reviews.apache.org/r/71375/diff/1/ > > > Testing > --- > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 71361: Added missing `return` statement in `Slave::statusUpdate`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71361/#review217426 --- Ship it! Ship It! - Gilbert Song On Aug. 23, 2019, 5:57 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71361/ > --- > > (Updated Aug. 23, 2019, 5:57 a.m.) > > > Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang. > > > Repository: mesos > > > Description > --- > > Previously, if `statusUpdate` was called for a pending task, it would > forward the status update and then continue executing `statusUpdate`, > which then checks if there is an executor that is aware of this task. > Given that a pending task is not known to any executor, it would always > handle it by forwarding status update one more time. This patch adds > missing `return` statement, which fixes the issue. > > > Diffs > - > > src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 > > > Diff: https://reviews.apache.org/r/71361/diff/1/ > > > Testing > --- > > `sudo make check` > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71343: Fixed out-of-order processing of terminal status updates in agent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71343/#review217419 --- Fix it, then Ship it! src/slave/slave.cpp Lines 10775-10776 (patched) <https://reviews.apache.org/r/71343/#comment304700> It does not seem likely to crash. But to be safe, could we consider to relax these CHECKs? E.g., just log a warning and return? - Gilbert Song On Aug. 21, 2019, 10:53 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71343/ > --- > > (Updated Aug. 21, 2019, 10:53 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, Mesos agent could send TASK_FAILED status update on > executor termination while processing of TASK_FINISHED status update > was in progress. Processing of task status updates involves sending > requests to the containerizer, which might finish processing of these > requests out-of-order, e.g. `MesosContainerizer::status`. Also, > the agent does not overwrite status of the terminal status update once > it's stored in the `terminatedTasks`. Hence, there was a race condition > between two terminal status updates. > > Note that V1 Executors are not affected by this problem because they > wait for an acknowledgement of the terminal status update by the agent > before terminating. > > This patch introduces a new data structure `pendingStatusUpdates`, > which holds a list of status updates that are being processed. This > data structure allows validating the order of processing of status > updates by the agent. > > > Diffs > - > > src/slave/slave.hpp a17bbee13cb8291ad694f1520b613764b57b046b > src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 > > > Diff: https://reviews.apache.org/r/71343/diff/2/ > > > Testing > --- > > 1. manual testing described in MESOS-9887 > 2. internal CI > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71335: Used cached cgroups for updating resources in Docker containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71335/#review217394 --- Ship it! Ship It! - Gilbert Song On Aug. 21, 2019, 5:24 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71335/ > --- > > (Updated Aug. 21, 2019, 5:24 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9836 > https://issues.apache.org/jira/browse/MESOS-9836 > > > Repository: mesos > > > Description > --- > > Used cached cgroups for updating resources in Docker containerizer. > > > Diffs > - > > src/slave/containerizer/docker.hpp ca41f3b6f48f583b0aa1eb07df01d3872b80fa6c > src/slave/containerizer/docker.cpp e4ad94572d34d49d599197afc08fd937253ecb0f > src/tests/containerizer/docker_containerizer_tests.cpp > e25acfda336f10b2e3d6a79d1a336a290dc5b407 > > > Diff: https://reviews.apache.org/r/71335/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71174/#review217339 --- Fix it, then Ship it! src/slave/containerizer/mesos/isolators/network/cni/cni.cpp Lines 542-547 (original), 542-556 (patched) <https://reviews.apache.org/r/71174/#comment304633> We distinguish the top level container from nested container by has_executor_info and has_container_info. Please add comments because people may not know it if they don't check ContainerState proto. - Gilbert Song On July 29, 2019, 1:40 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71174/ > --- > > (Updated July 29, 2019, 1:40 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9909 > https://issues.apache.org/jira/browse/MESOS-9909 > > > Repository: mesos > > > Description > --- > > Recovered network info for nested/standalone containers in CNI isolator. > > > Diffs > - > > include/mesos/slave/containerizer.proto > a60c96302a6cec90ecd0a0885b844fff8d37db71 > src/common/protobuf_utils.hpp 5d6a35d6e3bae35b83e87827724206f7c5dfb2d8 > src/common/protobuf_utils.cpp 7778e7f2475e9d6125d1c599715c91715f3654d3 > src/slave/containerizer/mesos/containerizer.cpp > a01edc8793a2eaa655f1729a01a01f1f61fbf7cb > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5 > > > Diff: https://reviews.apache.org/r/71174/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71287: Added a test `SlaveTest.DefaultExecutorResources`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71287/#review217231 --- Ship it! Ship It! - Gilbert Song On Aug. 14, 2019, 12:54 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71287/ > --- > > (Updated Aug. 14, 2019, 12:54 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9925 > https://issues.apache.org/jira/browse/MESOS-9925 > > > Repository: mesos > > > Description > --- > > Added a test `SlaveTest.DefaultExecutorResources`. > > > Diffs > - > > src/tests/slave_tests.cpp 1a926029a9f81eb0c43e9cddd73c318557760c27 > > > Diff: https://reviews.apache.org/r/71287/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71244: Included task group's resources in the ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71244/#review217230 --- Ship it! Ship It! - Gilbert Song On Aug. 14, 2019, 12:52 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71244/ > --- > > (Updated Aug. 14, 2019, 12:52 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9925 > https://issues.apache.org/jira/browse/MESOS-9925 > > > Repository: mesos > > > Description > --- > > Included task group's resources in the ExecutorInfo. > > > Diffs > - > > src/slave/slave.cpp 74eb45744d6603b91676e812ed008a7b1ab39a49 > > > Diff: https://reviews.apache.org/r/71244/diff/2/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71174/#review217229 --- Is it possible to avoid adding container_info in state? This is a public facing API change. No longer can be chagned again once public module start adopting it - Gilbert Song On July 29, 2019, 1:40 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71174/ > --- > > (Updated July 29, 2019, 1:40 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9909 > https://issues.apache.org/jira/browse/MESOS-9909 > > > Repository: mesos > > > Description > --- > > Recovered network info for nested/standalone containers in CNI isolator. > > > Diffs > - > > include/mesos/slave/containerizer.proto > a60c96302a6cec90ecd0a0885b844fff8d37db71 > src/common/protobuf_utils.hpp 5d6a35d6e3bae35b83e87827724206f7c5dfb2d8 > src/common/protobuf_utils.cpp 7778e7f2475e9d6125d1c599715c91715f3654d3 > src/slave/containerizer/mesos/containerizer.cpp > a01edc8793a2eaa655f1729a01a01f1f61fbf7cb > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5 > > > Diff: https://reviews.apache.org/r/71174/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71286: Added container transition times to the logs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71286/#review217228 --- Ship it! Ship It! - Gilbert Song On Aug. 13, 2019, 11:12 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71286/ > --- > > (Updated Aug. 13, 2019, 11:12 p.m.) > > > Review request for mesos and Gilbert Song. > > > Repository: mesos > > > Description > --- > > It's very useful to know directly how long a container spent in each > state. Capture a timestamp at each state transition time and log the > elapsed time in the corresponding state. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.hpp > 271d6326e9739634b9e2b5edece3ace20f3a150c > src/slave/containerizer/mesos/containerizer.cpp > 75e7cdac499692be6838aa122578129d9cfa724c > > > Diff: https://reviews.apache.org/r/71286/diff/2/ > > > Testing > --- > > make check > > > Thanks, > > James Peach > >
Re: Review Request 71222: Added a test `VolumeSecretIsolatorCleanupTest.ROOT_FailInPreparing`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71222/#review217227 --- Ship it! Ship It! - Gilbert Song On Aug. 1, 2019, 12:18 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71222/ > --- > > (Updated Aug. 1, 2019, 12:18 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9893 > https://issues.apache.org/jira/browse/MESOS-9893 > > > Repository: mesos > > > Description > --- > > Added a test `VolumeSecretIsolatorCleanupTest.ROOT_FailInPreparing`. > > > Diffs > - > > src/tests/containerizer/volume_secret_isolator_tests.cpp > b6c43c3c3fcc61cd1bd03725cde78e58bcff801e > > > Diff: https://reviews.apache.org/r/71222/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71221: Moved const string `.secret` to paths.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71221/#review217225 --- Ship it! Ship It! - Gilbert Song On Aug. 1, 2019, 12:16 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71221/ > --- > > (Updated Aug. 1, 2019, 12:16 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9893 > https://issues.apache.org/jira/browse/MESOS-9893 > > > Repository: mesos > > > Description > --- > > Moved const string `.secret` to paths.hpp. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/volume/secret.cpp > 4bbcc7af0216a483d71b367c154c24500545a40b > src/slave/containerizer/mesos/paths.hpp > c0033354bb07c68e4dda3cd49ca2d23164343e79 > > > Diff: https://reviews.apache.org/r/71221/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71201: Implemented `cleanup` method for `volume/secret` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71201/#review217224 --- Ship it! Ship It! - Gilbert Song On July 31, 2019, 12:04 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71201/ > --- > > (Updated July 31, 2019, 12:04 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9893 > https://issues.apache.org/jira/browse/MESOS-9893 > > > Repository: mesos > > > Description > --- > > Previously, after `volume/secret` isolator resolves a secret and write > it into a path (i.e., /.secret/) on agent host for a > container, if the container fails to launch somehow (e.g., fails in > another isolator's `prepare` method), that path on the host will never > be cleaned up. In this patch, `volume/secret` isolator is improved to > write all the resolved secrets for a container into a single directory > (i.e., /.secret/) on agent host, and the > `cleanup` method of the `volume/secret` isolator is implemented to > remove that directory when the container is destroyed. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/volume/secret.hpp > a1664915bc6c5ff223fcc9949448408883c3010c > src/slave/containerizer/mesos/isolators/volume/secret.cpp > 4bbcc7af0216a483d71b367c154c24500545a40b > > > Diff: https://reviews.apache.org/r/71201/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71283: Made sure we are tracking ephemeral quota before getting the usage.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71283/#review217194 --- Ship it! Ship It! - Gilbert Song On Aug. 13, 2019, 3:59 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71283/ > --- > > (Updated Aug. 13, 2019, 3:59 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9935 > https://issues.apache.org/jira/browse/MESOS-9935 > > > Repository: mesos > > > Description > --- > > If there is no ephemeral disk resource, the `disk/du` isolator > will not track ephemeral disk usage. In that case, we can't index > the paths hash to find the last usage since it doesn't contain the > ephemeral paths. The fix is to check, and omit the ephemeral usage > if it is not being tracked. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/posix/disk.cpp > 29bdbe6ccffd755df4dc48983c5f99fc2f0ae5d2 > > > Diff: https://reviews.apache.org/r/71283/diff/1/ > > > Testing > --- > > make check with empty disk resources > > > Thanks, > > James Peach > >
Re: Review Request 71247: Added a new agent flag --allow_chown_docker_volume.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71247/ --- (Updated Aug. 8, 2019, 4:55 p.m.) Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Added a new agent flag --allow_chown_docker_volume. Diffs (updated) - docs/configuration/agent.md 325a37e095f861f11929218aa5c873b2c714b490 src/slave/flags.hpp 01834f41f2cd045cdb745e90d7f57fc2948ae024 src/slave/flags.cpp 08ec20b30e7d82ab453c0b96c208ab1c1cc812db Diff: https://reviews.apache.org/r/71247/diff/2/ Changes: https://reviews.apache.org/r/71247/diff/1-2/ Testing --- make Thanks, Gilbert Song
Re: Review Request 71248: Supported chown docker volumes in the docker volume isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71248/ --- (Updated Aug. 8, 2019, 4:55 p.m.) Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description (updated) --- If the agent flag --docker_volume_chown is true, Mesos will chown the docker volume to the container user non-recursively. Diffs (updated) - src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 40119d9bc6842e326792a7c1dc1b49b17ee30f6e Diff: https://reviews.apache.org/r/71248/diff/3/ Changes: https://reviews.apache.org/r/71248/diff/2-3/ Testing --- make Thanks, Gilbert Song
Re: Review Request 71249: Added a unit test for the docker volume chown support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71249/ --- (Updated Aug. 8, 2019, 4:55 p.m.) Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Added a unit test for the docker volume chown support. Diffs (updated) - src/tests/containerizer/docker_volume_isolator_tests.cpp cd5c028991b0ef2438dd5c61a08d48747a109585 Diff: https://reviews.apache.org/r/71249/diff/3/ Changes: https://reviews.apache.org/r/71249/diff/2-3/ Testing --- make check Thanks, Gilbert Song
Re: Review Request 71249: Added a unit test for the docker volume chown support.
> On Aug. 8, 2019, 1:33 a.m., Qian Zhang wrote: > > src/tests/containerizer/docker_volume_isolator_tests.cpp > > Lines 1275 (patched) > > <https://reviews.apache.org/r/71249/diff/2/?file=2160249#file2160249line1275> > > > > I think we need `UNPRIVILEGED_USER_` in the test name to use > > `SUDO_USER` in the test. And then we do not need `NonRootUser` in the test > > name. I was trying to use that prefix but it makes it too long. - Gilbert --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71249/#review217118 ------- On Aug. 7, 2019, 11:59 p.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71249/ > --- > > (Updated Aug. 7, 2019, 11:59 p.m.) > > > Review request for mesos, Andrei Budnik and Qian Zhang. > > > Bugs: MESOS-9908 > https://issues.apache.org/jira/browse/MESOS-9908 > > > Repository: mesos > > > Description > --- > > Added a unit test for the docker volume chown support. > > > Diffs > - > > src/tests/containerizer/docker_volume_isolator_tests.cpp > cd5c028991b0ef2438dd5c61a08d48747a109585 > > > Diff: https://reviews.apache.org/r/71249/diff/2/ > > > Testing > --- > > make check > > > Thanks, > > Gilbert Song > >
Re: Review Request 71247: Added a new agent flag --allow_chown_docker_volume.
> On Aug. 7, 2019, 11:43 p.m., Qian Zhang wrote: > > I think we also need to update `agent.md`, `operator-http-api.md` (the > > `GET_FLAGS` call), `upgrades.md` and `CHANGELOG` for this flag. Updated agent.md I will update other .md file in a separate patch. Thanks! - Gilbert --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71247/#review217116 --- On Aug. 7, 2019, 5:54 p.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71247/ > --- > > (Updated Aug. 7, 2019, 5:54 p.m.) > > > Review request for mesos, Andrei Budnik and Qian Zhang. > > > Bugs: MESOS-9908 > https://issues.apache.org/jira/browse/MESOS-9908 > > > Repository: mesos > > > Description > --- > > Added a new agent flag --allow_chown_docker_volume. > > > Diffs > - > > src/slave/flags.hpp 01834f41f2cd045cdb745e90d7f57fc2948ae024 > src/slave/flags.cpp 08ec20b30e7d82ab453c0b96c208ab1c1cc812db > > > Diff: https://reviews.apache.org/r/71247/diff/1/ > > > Testing > --- > > make > > > Thanks, > > Gilbert Song > >
Re: Review Request 71249: Added a unit test for the docker volume chown support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71249/ --- (Updated Aug. 7, 2019, 11:59 p.m.) Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Added a unit test for the docker volume chown support. Diffs (updated) - src/tests/containerizer/docker_volume_isolator_tests.cpp cd5c028991b0ef2438dd5c61a08d48747a109585 Diff: https://reviews.apache.org/r/71249/diff/2/ Changes: https://reviews.apache.org/r/71249/diff/1-2/ Testing --- make check Thanks, Gilbert Song
Re: Review Request 71248: Supported chown docker volumes in the docker volume isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71248/ --- (Updated Aug. 7, 2019, 11:59 p.m.) Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Supported chown docker volumes in the docker volume isolator. Diffs (updated) - src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 40119d9bc6842e326792a7c1dc1b49b17ee30f6e Diff: https://reviews.apache.org/r/71248/diff/2/ Changes: https://reviews.apache.org/r/71248/diff/1-2/ Testing --- make Thanks, Gilbert Song
Review Request 71248: Supported chown docker volumes in the docker volume isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71248/ --- Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Supported chown docker volumes in the docker volume isolator. Diffs - src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 40119d9bc6842e326792a7c1dc1b49b17ee30f6e Diff: https://reviews.apache.org/r/71248/diff/1/ Testing --- make Thanks, Gilbert Song
Review Request 71249: Added a unit test for the docker volume chown support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71249/ --- Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Added a unit test for the docker volume chown support. Diffs - src/tests/containerizer/docker_volume_isolator_tests.cpp cd5c028991b0ef2438dd5c61a08d48747a109585 Diff: https://reviews.apache.org/r/71249/diff/1/ Testing --- make check Thanks, Gilbert Song
Review Request 71247: Added a new agent flag --allow_chown_docker_volume.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71247/ --- Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-9908 https://issues.apache.org/jira/browse/MESOS-9908 Repository: mesos Description --- Added a new agent flag --allow_chown_docker_volume. Diffs - src/slave/flags.hpp 01834f41f2cd045cdb745e90d7f57fc2948ae024 src/slave/flags.cpp 08ec20b30e7d82ab453c0b96c208ab1c1cc812db Diff: https://reviews.apache.org/r/71247/diff/1/ Testing --- make Thanks, Gilbert Song
Re: Review Request 71235: Added a test `ROOT_VETH_VerifyNestedContainerIPAfterReboot`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71235/#review217095 --- Ship it! Ship It! - Gilbert Song On Aug. 3, 2019, 12:24 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71235/ > --- > > (Updated Aug. 3, 2019, 12:24 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9868 > https://issues.apache.org/jira/browse/MESOS-9868 > > > Repository: mesos > > > Description > --- > > Added a test `ROOT_VETH_VerifyNestedContainerIPAfterReboot`. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp > 0fc664b968485ca96444e186e58190eadc660830 > > > Diff: https://reviews.apache.org/r/71235/diff/1/ > > > Testing > --- > > Ran this test repeatedly and all the iterations succeeded. > > This test would fail without the previous patch > https://reviews.apache.org/r/71172/ . > > > Thanks, > > Qian Zhang > >
Re: Review Request 71172: Looked up parent's IP for the non-checkpointed nested containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71172/#review217094 --- Ship it! Ship It! - Gilbert Song On July 29, 2019, 1:40 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71172/ > --- > > (Updated July 29, 2019, 1:40 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9868 > https://issues.apache.org/jira/browse/MESOS-9868 > > > Repository: mesos > > > Description > --- > > Looked up parent's IP for the non-checkpointed nested containers. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5 > > > Diff: https://reviews.apache.org/r/71172/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71172: Looked up parent's IP for the non-checkpointed nested containers.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review216976
---
Ship it!
This part of code is fragile. Probably we need a unit test.
The complication comes from the factor that we only checkpointed partial
information. Probably we should just check point all containers.
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1445-1457 (original), 1447-1472 (patched)
<https://reviews.apache.org/r/71172/#comment304202>
Can review this patch https://reviews.apache.org/r/65987/ again and make
sure there is no similar logic mistake in other methods
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1445-1457 (original), 1447-1472 (patched)
<https://reviews.apache.org/r/71172/#comment304203>
another option is:
```
if (isNestedContainer) {
if (!infos.contains(containerId) ||
infos[containerId]->joinsParentsNetwork) {
return status(containerId.parent());
}
}
if (!infos.contains(containerId)) {
return ContainerStatus();
}
```
up to you to pick the style that is easier to read.
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1448-1461 (patched)
<https://reviews.apache.org/r/71172/#comment304201>
if a nested container has its own network namespace, I guess its INFO will
be checkpointed?
- Gilbert Song
On July 29, 2019, 1:40 a.m., Qian Zhang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> ---
>
> (Updated July 29, 2019, 1:40 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9868
> https://issues.apache.org/jira/browse/MESOS-9868
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> ---
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71003: Added a test `DefaultExecutorTest.AllocationRoleEnvironmentVariable`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71003/#review216944 --- Ship it! Ship It! - Gilbert Song On July 3, 2019, 7:08 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71003/ > --- > > (Updated July 3, 2019, 7:08 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9874 > https://issues.apache.org/jira/browse/MESOS-9874 > > > Repository: mesos > > > Description > --- > > Added a test `DefaultExecutorTest.AllocationRoleEnvironmentVariable`. > > > Diffs > - > > src/tests/default_executor_tests.cpp > 1c3b48848cf387fe9fb109abfaffa5be04e2a3e1 > > > Diff: https://reviews.apache.org/r/71003/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71002: Added a test `CommandExecutorTest.AllocationRoleEnvironmentVariable`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71002/#review216943 --- Ship it! Ship It! - Gilbert Song On July 3, 2019, 7:07 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71002/ > --- > > (Updated July 3, 2019, 7:07 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9874 > https://issues.apache.org/jira/browse/MESOS-9874 > > > Repository: mesos > > > Description > --- > > Added a test `CommandExecutorTest.AllocationRoleEnvironmentVariable`. > > > Diffs > - > > src/tests/command_executor_tests.cpp > 02ae250a599043f30ef5879ce528815e0ded5d3d > > > Diff: https://reviews.apache.org/r/71002/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71004: Added a test `ROOT_DOCKER_AllocationRoleEnvironmentVariable`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71004/#review216945 --- Ship it! Ship It! - Gilbert Song On July 3, 2019, 7:09 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71004/ > --- > > (Updated July 3, 2019, 7:09 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9874 > https://issues.apache.org/jira/browse/MESOS-9874 > > > Repository: mesos > > > Description > --- > > Added a test `ROOT_DOCKER_AllocationRoleEnvironmentVariable`. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > a6217581e20168c5114f733323e927a83ac59fd0 > > > Diff: https://reviews.apache.org/r/71004/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71136: Fixed use-after-free bug in `PendingFutureTrackerProcess`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71136/#review216792 --- Ship it! Ship It! - Gilbert Song On July 22, 2019, 8:51 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71136/ > --- > > (Updated July 22, 2019, 8:51 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, Meng Zhu, and Qian > Zhang. > > > Repository: mesos > > > Description > --- > > Previously, the future tracker subscribed on `onAny`, `onAbandoned`, > `onDiscard` events for the future using callbacks, which accept > the same iterator to the `pending` list. Since `onAny` and `onDiscard` > events can overlap, the same callback can be called more than once, > which leads to accessing invalidated iterator. This patch fixes the > problem by removing `onDiscard` handler because it does not transition > a future to the terminal state. > > > Diffs > - > > src/common/future_tracker.hpp a3f191a58b7ada58153ca33a8d0409b846faedae > > > Diff: https://reviews.apache.org/r/71136/diff/2/ > > > Testing > --- > > I was able to reproduce a segfault observed in our CI by launching > `ROOT_LaunchGroupFailure` test in the loop with `stress` running in parallel: > ``` > sudo src/mesos-tests --verbose --gtest_repeat=100 --break_on_failure > --gtest_filter=ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.ROOT_LaunchGroupFailure/0 > > stress -c 16 > ``` > After applying the patch, the test passed successfully. > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71136: Fixed use-after-free bug in `PendingFutureTrackerProcess`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71136/#review216791 --- src/common/future_tracker.hpp Lines 67-68 (original), 73-74 (patched) <https://reviews.apache.org/r/71136/#comment304015> could we consider to remove .onDiscard()? I believe the registered future or other futures chained in the containerizer already have onDiscard handler, which means that the future will somehow transit to terminated state or a terminated state will be propagated back to this future if someone calls .discard() or from a discard is called by a HTTP disconnection. If that is the case, .onAny + .onAbandoned should be sufficient. wdyt? - Gilbert Song On July 22, 2019, 8:51 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71136/ > --- > > (Updated July 22, 2019, 8:51 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, Meng Zhu, and Qian > Zhang. > > > Repository: mesos > > > Description > --- > > Previously, the future tracker subscribed on `onAny`, `onAbandoned`, > `onDiscard` events for the future using callbacks, which accept > the same iterator to the `pending` list. Since above mentioned events > can overlap, the same callback can be called more than once, which > leads to accessing invalidated iterator. This patch fixes the problem > by introducing `erased` flag. > > > Diffs > - > > src/common/future_tracker.hpp a3f191a58b7ada58153ca33a8d0409b846faedae > > > Diff: https://reviews.apache.org/r/71136/diff/1/ > > > Testing > --- > > I was able to reproduce a segfault observed in our CI by launching > `ROOT_LaunchGroupFailure` test in the loop with `stress` running in parallel: > ``` > sudo src/mesos-tests --verbose --gtest_repeat=100 --break_on_failure > --gtest_filter=ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.ROOT_LaunchGroupFailure/0 > > stress -c 16 > ``` > After applying the patch, the test passed successfully. > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71121: Added a test `ROOT_NonePrivateIPCModeWithShmSize`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71121/#review216780 --- Ship it! Ship It! - Gilbert Song On July 19, 2019, 3:42 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71121/ > --- > > (Updated July 19, 2019, 3:42 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added a test `ROOT_NonePrivateIPCModeWithShmSize`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > 14feaed50d4e71e2dfaba21880cd67f53584366f > > > Diff: https://reviews.apache.org/r/71121/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71122: Added a test `ROOT_DebugContainerWithPrivateIPCMode`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71122/#review216781 --- Ship it! Ship It! - Gilbert Song On July 19, 2019, 3:42 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71122/ > --- > > (Updated July 19, 2019, 3:42 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added a test `ROOT_DebugContainerWithPrivateIPCMode`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > 14feaed50d4e71e2dfaba21880cd67f53584366f > > > Diff: https://reviews.apache.org/r/71122/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 71120: Added two validations in `namespaces/ipc` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71120/#review216779 --- Ship it! Ship It! - Gilbert Song On July 19, 2019, 3:42 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71120/ > --- > > (Updated July 19, 2019, 3:42 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > 1. Do not support specifying the size of /dev/shm when the IPC mode >is not `PRIVATE`. > 2. Do not support private IPC mode for debug containers. > > > Diffs > - > > docs/isolators/namespaces-ipc.md 4c978ec4995538b45fc5de6db199b6e3af6a2e99 > include/mesos/mesos.proto 075c110acb9977e67a87220cf97e7e325826b2af > include/mesos/v1/mesos.proto 0dcaee659764b26c548aba6827404a5577491db0 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 743d48d07cdb3729d871dcb1c74d5d841af5f53f > src/tests/containerizer/isolator_tests.cpp > 14feaed50d4e71e2dfaba21880cd67f53584366f > > > Diff: https://reviews.apache.org/r/71120/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71128: Fixed an UI issue in Roles tab where `Reserved` is incorrect.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71128/#review216777 --- Ship it! Ship It! - Gilbert Song On July 19, 2019, 11:56 a.m., Meng Zhu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71128/ > --- > > (Updated July 19, 2019, 11:56 a.m.) > > > Review request for mesos, Benjamin Mahler and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Fixed an UI issue in Roles tab where `Reserved` is incorrect. > > > Diffs > - > > src/webui/app/roles/roles.html b0c57cf9c9f9a6cf8c595e3efcfcd990b2b8d6ac > > > Diff: https://reviews.apache.org/r/71128/diff/1/ > > > Testing > --- > > See screenshot. > > > File Attachments > > > after fix > > https://reviews.apache.org/media/uploaded/files/2019/07/19/f825a9ea-a452-4b40-ac61-d5772ff203ad__Screen_Shot_2019-07-19_at_11.53.12_AM.png > > > Thanks, > > Meng Zhu > >
Re: Review Request 71065: Implemented `FutureTrackTest` tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71065/#review216730 --- Ship it! Ship It! - Gilbert Song On July 12, 2019, 9:03 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71065/ > --- > > (Updated July 12, 2019, 9:03 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9842 > https://issues.apache.org/jira/browse/MESOS-9842 > > > Repository: mesos > > > Description > --- > > These tests verify functionality provided by the > `PendingFutureTracker` class. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db > src/tests/common/future_tracker_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/71065/diff/4/ > > > Testing > --- > > `src/mesos-tests --gtest_repeat=100 --gtest_filter=FutureTrackTest.*` > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70892: Added `/containerizer/debug` endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70892/#review216729
---
Ship it!
Ship It!
- Gilbert Song
On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70892/
> ---
>
> (Updated June 19, 2019, 7:49 a.m.)
>
>
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9829
> https://issues.apache.org/jira/browse/MESOS-9829
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch introduces an agent's `/containerizer/debug` endpoint,
> which exposes the debug info related to Mesos containerizer.
> Currently, this endpoint returns a list of pending futures related to
> isolators or containerizer launcher. This endpoint is experimental,
> and the format of its output may change over time.
>
>
> Diffs
> -
>
> docs/authorization.md 91dc03be66bdcf135610bff50e5abcfe8d95c5db
> src/common/authorization.cpp 89487219ef87619084663e1a0a17e75924394d38
> src/local/local.cpp 6ac6b027f66ef555440ca86803198bfe3227f8d7
> src/slave/http.hpp b8c83f1db95c2175bb94f6cd76cc2c58aa118c4e
> src/slave/http.cpp 321dca786f106169f88df086422f5ef817d2f098
> src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6
> src/slave/slave.hpp 58a5608ceb53d7f75a085f1d3a4f757e8c3ddd3b
> src/slave/slave.cpp 0e7e4d450b77f48a42debf47cb6fc12fdde5858b
> src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5
> src/tests/mock_slave.hpp c057b40d6db433f52d1dfe10ae02a3b0bc936564
> src/tests/mock_slave.cpp dd458e3d09e212d6cde514d86989878954f79fc1
>
>
> Diff: https://reviews.apache.org/r/70892/diff/5/
>
>
> Testing
> ---
>
> Manual testing.
> 1. Add `::sleep(5)` to the beginning of the
> `LinuxSeccompIsolatorProcess::prepare` method.
> 2. Rebuild Mesos and then spin up a local Mesos cluster.
> 3. Run simple task, e.g.:
> ```
> ./src/mesos-execute --master="`hostname`:5050" --name="test"
> --containerizer=mesos --command="sleep 1"
> ```
> 4. In the parallel terminal session:
> ```
> curl `hostname`:5051/containerizer/debug
> # will return:
> {"pending":[["linux/seccomp::prepare",{"containerId":""}]]}
> ```
> 5. After 5 seconds this endpoint returns an empty list of pending futures:
> `{"pending":[]}`
>
> Unit tests: TBD in the following patches.
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70890/#review216728 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70890/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9840 > https://issues.apache.org/jira/browse/MESOS-9840 > > > Repository: mesos > > > Description > --- > > This patch adds a new `LauncherTracker` class that proxies > calls to the real `Launcher` and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt 6d1409003c4727f6a627f0c0f20d65c2db623012 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70890/diff/4/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70889: Wrapped isolators in `IsolatorTracker`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70889/#review216727 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70889/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9841 > https://issues.apache.org/jira/browse/MESOS-9841 > > > Repository: mesos > > > Description > --- > > This patch wraps every isolator in instance of `IsolatorTracker` class. > If an isolator gets stuck in some operation, `pendingFutures` will > return the isolator name, method name along with its arguments such as > `containerId`, `pid`, etc. > > > Diffs > - > > src/slave/containerizer/containerizer.hpp > d33c65cd3fae9bb4d347681cb00dbdb25ecf57b8 > src/slave/containerizer/containerizer.cpp > 5ce0d9c4bfa707495ea644c90fa6e35eb0fe25cd > src/slave/containerizer/mesos/containerizer.hpp > 558e412b28a84ae0ca1bf05b596540072bb216d6 > src/slave/containerizer/mesos/containerizer.cpp > c9a369bcdbfc1676912430c3e84fa567bbd7f766 > src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6 > src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5 > > > Diff: https://reviews.apache.org/r/70889/diff/6/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70888/#review216726 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70888/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9839 > https://issues.apache.org/jira/browse/MESOS-9839 > > > Repository: mesos > > > Description > --- > > This patch adds a new `IsolatorTracker` class that proxies > calls to the real isolator and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac > src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70888/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70887/#review216725 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70887/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng > Zhu, and Qian Zhang. > > > Bugs: MESOS-9837 > https://issues.apache.org/jira/browse/MESOS-9837 > > > Repository: mesos > > > Description > --- > > This patch introduces a mechanism for tracking pending futures. > This feature allows detection of hanging operations, which get > stuck on a blocking operation or asynchronously. However, this > feature does not provide any mechanism for tracking pending > promises, because `Promise` objects might not be accessible in > various cases. Thereby, we introduce a new class that can be > used to track pending futures, so it might facilitate debugging > of stuck issues. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/common/future_tracker.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70887/diff/7/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70890/#review216710
---
src/slave/containerizer/mesos/launcher_tracker.cpp
Lines 62-80 (patched)
<https://reviews.apache.org/r/70890/#comment303901>
Could we avoid returning future.get(). It is not a risk here but not very
readable.
We could rely on a promise:
```
Promise promise;
tracker->track(
promise.future(),
"launcher::fork",
COMPONENT_NAME_CONTAINERIZER,
{{"containerId", stringify(containerId)},
{"path", path}});
Try forked = launcher->fork(
containerId,
path,
argv,
containerIO,
flags,
environment,
enterNamespaces,
cloneNamespaces,
whitelistFds);
promise.associate(forked);
return forked;
```
- Gilbert Song
On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70890/
> ---
>
> (Updated June 19, 2019, 7:49 a.m.)
>
>
> Review request for mesos, Gilbert Song and Qian Zhang.
>
>
> Bugs: MESOS-9840
> https://issues.apache.org/jira/browse/MESOS-9840
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch adds a new `LauncherTracker` class that proxies
> calls to the real `Launcher` and keeps track of returned futures
> by employing `PendingFutureTracker` class.
>
>
> Diffs
> -
>
> src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
> src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
> src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION
> src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70890/diff/3/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 71065: Implemented `FutureTrackTest` tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71065/#review216685 --- Ship it! Ship It! - Gilbert Song On July 12, 2019, 9:03 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71065/ > --- > > (Updated July 12, 2019, 9:03 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9842 > https://issues.apache.org/jira/browse/MESOS-9842 > > > Repository: mesos > > > Description > --- > > These tests verify functionality provided by the > `PendingFutureTracker` class. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db > src/tests/common/future_tracker_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/71065/diff/3/ > > > Testing > --- > > `src/mesos-tests --gtest_repeat=100 --gtest_filter=FutureTrackTest.*` > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70892: Added `/containerizer/debug` endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70892/#review216684
---
Ship it!
Ship It!
- Gilbert Song
On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70892/
> ---
>
> (Updated June 19, 2019, 7:49 a.m.)
>
>
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9829
> https://issues.apache.org/jira/browse/MESOS-9829
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch introduces an agent's `/containerizer/debug` endpoint,
> which exposes the debug info related to Mesos containerizer.
> Currently, this endpoint returns a list of pending futures related to
> isolators or containerizer launcher. This endpoint is experimental,
> and the format of its output may change over time.
>
>
> Diffs
> -
>
> docs/authorization.md 91dc03be66bdcf135610bff50e5abcfe8d95c5db
> src/common/authorization.cpp 89487219ef87619084663e1a0a17e75924394d38
> src/local/local.cpp 6ac6b027f66ef555440ca86803198bfe3227f8d7
> src/slave/http.hpp b8c83f1db95c2175bb94f6cd76cc2c58aa118c4e
> src/slave/http.cpp 321dca786f106169f88df086422f5ef817d2f098
> src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6
> src/slave/slave.hpp 58a5608ceb53d7f75a085f1d3a4f757e8c3ddd3b
> src/slave/slave.cpp 247797543d9608fb212b5f09865f744052a460e3
> src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5
> src/tests/mock_slave.hpp c057b40d6db433f52d1dfe10ae02a3b0bc936564
> src/tests/mock_slave.cpp dd458e3d09e212d6cde514d86989878954f79fc1
>
>
> Diff: https://reviews.apache.org/r/70892/diff/4/
>
>
> Testing
> ---
>
> Manual testing.
> 1. Add `::sleep(5)` to the beginning of the
> `LinuxSeccompIsolatorProcess::prepare` method.
> 2. Rebuild Mesos and then spin up a local Mesos cluster.
> 3. Run simple task, e.g.:
> ```
> ./src/mesos-execute --master="`hostname`:5050" --name="test"
> --containerizer=mesos --command="sleep 1"
> ```
> 4. In the parallel terminal session:
> ```
> curl `hostname`:5051/containerizer/debug
> # will return:
> {"pending":[["linux/seccomp::prepare",{"containerId":""}]]}
> ```
> 5. After 5 seconds this endpoint returns an empty list of pending futures:
> `{"pending":[]}`
>
> Unit tests: TBD in the following patches.
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 70891: Wrapped launcher in `LauncherTracker`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70891/#review216683 --- Fix it, then Ship it! src/slave/containerizer/mesos/containerizer.cpp Lines 350 (patched) <https://reviews.apache.org/r/70891/#comment303886> isn't this always true? - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70891/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9841 > https://issues.apache.org/jira/browse/MESOS-9841 > > > Repository: mesos > > > Description > --- > > This patch wraps a container launcher in instance of `LauncherTracker` > class. If the launcher gets stuck in some operation, `pendingFutures` > will return the method name along with its arguments such as > `containerId`, `pid`, etc. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > c9a369bcdbfc1676912430c3e84fa567bbd7f766 > > > Diff: https://reviews.apache.org/r/70891/diff/2/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70890/#review216682 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70890/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9840 > https://issues.apache.org/jira/browse/MESOS-9840 > > > Repository: mesos > > > Description > --- > > This patch adds a new `LauncherTracker` class that proxies > calls to the real `Launcher` and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70890/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70889: Wrapped isolators in `IsolatorTracker`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70889/#review216681
---
Fix it, then Ship it!
src/slave/containerizer/mesos/containerizer.cpp
Lines 550 (patched)
<https://reviews.apache.org/r/70889/#comment303884>
could we be explicit here?
if (futuretracker != nullptr)
src/slave/containerizer/mesos/containerizer.cpp
Lines 570 (patched)
<https://reviews.apache.org/r/70889/#comment303885>
ditto
- Gilbert Song
On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70889/
> ---
>
> (Updated June 19, 2019, 7:49 a.m.)
>
>
> Review request for mesos, Gilbert Song and Qian Zhang.
>
>
> Bugs: MESOS-9841
> https://issues.apache.org/jira/browse/MESOS-9841
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch wraps every isolator in instance of `IsolatorTracker` class.
> If an isolator gets stuck in some operation, `pendingFutures` will
> return the isolator name, method name along with its arguments such as
> `containerId`, `pid`, etc.
>
>
> Diffs
> -
>
> src/slave/containerizer/containerizer.hpp
> d33c65cd3fae9bb4d347681cb00dbdb25ecf57b8
> src/slave/containerizer/containerizer.cpp
> 5ce0d9c4bfa707495ea644c90fa6e35eb0fe25cd
> src/slave/containerizer/mesos/containerizer.hpp
> 558e412b28a84ae0ca1bf05b596540072bb216d6
> src/slave/containerizer/mesos/containerizer.cpp
> c9a369bcdbfc1676912430c3e84fa567bbd7f766
> src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6
> src/tests/cluster.hpp c04ee14beafe350568a295c8258566aa3704028a
> src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5
>
>
> Diff: https://reviews.apache.org/r/70889/diff/4/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70888/#review216677 --- Ship it! Ship It! - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70888/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9839 > https://issues.apache.org/jira/browse/MESOS-9839 > > > Repository: mesos > > > Description > --- > > This patch adds a new `IsolatorTracker` class that proxies > calls to the real isolator and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac > src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70888/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70887/#review216671 --- Fix it, then Ship it! src/common/future_tracker.hpp Lines 101-103 (patched) <https://reviews.apache.org/r/70887/#comment303868> Please add some comments if possible :) - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70887/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng > Zhu, and Qian Zhang. > > > Bugs: MESOS-9837 > https://issues.apache.org/jira/browse/MESOS-9837 > > > Repository: mesos > > > Description > --- > > This patch introduces a mechanism for tracking pending futures. > This feature allows detection of hanging operations, which get > stuck on a blocking operation or asynchronously. However, this > feature does not provide any mechanism for tracking pending > promises, because `Promise` objects might not be accessible in > various cases. Thereby, we introduce a new class that can be > used to track pending futures, so it might facilitate debugging > of stuck issues. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/common/future_tracker.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70887/diff/5/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70892: Added `/containerizer/debug` endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70892/#review216645
---
this patch lgtm! thx!
- Gilbert Song
On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70892/
> ---
>
> (Updated June 19, 2019, 7:49 a.m.)
>
>
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9829
> https://issues.apache.org/jira/browse/MESOS-9829
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch introduces an agent's `/containerizer/debug` endpoint,
> which exposes the debug info related to Mesos containerizer.
> Currently, this endpoint returns a list of pending futures related to
> isolators or containerizer launcher. This endpoint is experimental,
> and the format of its output may change over time.
>
>
> Diffs
> -
>
> src/local/local.cpp 6ac6b027f66ef555440ca86803198bfe3227f8d7
> src/slave/http.hpp b8c83f1db95c2175bb94f6cd76cc2c58aa118c4e
> src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4
> src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6
> src/slave/slave.hpp 6954f53ff1531b9fcb688ef76acddf6a3d849a41
> src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e
> src/tests/cluster.hpp c04ee14beafe350568a295c8258566aa3704028a
> src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5
> src/tests/mock_slave.hpp c057b40d6db433f52d1dfe10ae02a3b0bc936564
> src/tests/mock_slave.cpp dd458e3d09e212d6cde514d86989878954f79fc1
>
>
> Diff: https://reviews.apache.org/r/70892/diff/3/
>
>
> Testing
> ---
>
> Manual testing.
> 1. Add `::sleep(5)` to the beginning of the
> `LinuxSeccompIsolatorProcess::prepare` method.
> 2. Rebuild Mesos and then spin up a local Mesos cluster.
> 3. Run simple task, e.g.:
> ```
> ./src/mesos-execute --master="`hostname`:5050" --name="test"
> --containerizer=mesos --command="sleep 1"
> ```
> 4. In the parallel terminal session:
> ```
> curl `hostname`:5051/containerizer/debug
> # will return:
> {"pending":[["linux/seccomp::prepare",{"containerId":""}]]}
> ```
> 5. After 5 seconds this endpoint returns an empty list of pending futures:
> `{"pending":[]}`
>
> Unit tests: TBD in the following patches.
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70890/#review216644 --- seems like this is a tracker wrapper similar to the isolatorTracker. A quick question: if the a container launch gets stuck at systemd cgroup hierachy isolate, could our tracker process be able to tell it is systemd? - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70890/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9840 > https://issues.apache.org/jira/browse/MESOS-9840 > > > Repository: mesos > > > Description > --- > > This patch adds a new `LauncherTracker` class that proxies > calls to the real `Launcher` and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70890/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70889: Wrapped isolators in `IsolatorTracker`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70889/#review216643 --- src/slave/containerizer/mesos/containerizer.cpp Lines 550-552 (patched) <https://reviews.apache.org/r/70889/#comment303847> seems like we are going to have one more libprocess process for each isolator/module. we may have some concerns on perf. would you mind to collect some perf data for w/o isolatorTracker and compare? A quick method to test it out is to use mesos-executor and launch 100 containers in one single task group. src/slave/main.cpp Lines 494 (patched) <https://reviews.apache.org/r/70889/#comment303846> is it possible to construct the `PendingFutureTracker` process in containerizer::create() - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70889/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9841 > https://issues.apache.org/jira/browse/MESOS-9841 > > > Repository: mesos > > > Description > --- > > This patch wraps every isolator in instance of `IsolatorTracker` class. > If an isolator gets stuck in some operation, `pendingFutures` will > return the isolator name, method name along with its arguments such as > `containerId`, `pid`, etc. > > > Diffs > - > > src/slave/containerizer/containerizer.hpp > d33c65cd3fae9bb4d347681cb00dbdb25ecf57b8 > src/slave/containerizer/containerizer.cpp > 5ce0d9c4bfa707495ea644c90fa6e35eb0fe25cd > src/slave/containerizer/mesos/containerizer.hpp > 558e412b28a84ae0ca1bf05b596540072bb216d6 > src/slave/containerizer/mesos/containerizer.cpp > c9a369bcdbfc1676912430c3e84fa567bbd7f766 > src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6 > src/tests/cluster.hpp c04ee14beafe350568a295c8258566aa3704028a > src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5 > > > Diff: https://reviews.apache.org/r/70889/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70888/#review216642 --- src/slave/constants.hpp Lines 205-207 (patched) <https://reviews.apache.org/r/70888/#comment303844> are you using `COMPONENT_NAME_` prefix because `MESOS_CONTAINERIZER` is already defined in containerizer/mesos/constant.hpp? probably let's figure out what `string component` stand for first. Then, we could better define the constant, and I would probably suggest to have the constant in containerizer/mesos/constant.hpp because we may have docker container stuck issue and if the docker container use this tracker process we need to distinguish two containerizers. src/slave/containerizer/mesos/isolator_tracker.hpp Lines 71 (patched) <https://reviews.apache.org/r/70888/#comment303845> could this be a Owned pointer? - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70888/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Bugs: MESOS-9839 > https://issues.apache.org/jira/browse/MESOS-9839 > > > Repository: mesos > > > Description > --- > > This patch adds a new `IsolatorTracker` class that proxies > calls to the real isolator and keeps track of returned futures > by employing `PendingFutureTracker` class. > > > Diffs > - > > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac > src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION > src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70888/diff/3/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70887/#review216641 --- src/common/future_tracker.hpp Lines 101-105 (patched) <https://reviews.apache.org/r/70887/#comment303843> probably we want to add some comments to explain these parameters. Other than `name`, another para confuses me a bit is `component`. In next patch, we have `containerizer` as component and `isolator::method` as name. How do we define component and name for the following case: ``` mesos_containerizer->provisioner->docker_store->local_puller->pull()``` - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70887/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng > Zhu, and Qian Zhang. > > > Bugs: MESOS-9837 > https://issues.apache.org/jira/browse/MESOS-9837 > > > Repository: mesos > > > Description > --- > > This patch introduces a mechanism for tracking pending futures. > This feature allows detection of hanging operations, which get > stuck on a blocking operation or asynchronously. However, this > feature does not provide any mechanism for tracking pending > promises, because `Promise` objects might not be accessible in > various cases. Thereby, we introduce a new class that can be > used to track pending futures, so it might facilitate debugging > of stuck issues. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/common/future_tracker.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70887/diff/4/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70887/#review216640 --- src/common/future_tracker.hpp Lines 103 (patched) <https://reviews.apache.org/r/70887/#comment303842> is there a variable name better than `name`? so that easier for dev to understand this parameter. function or method? - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70887/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng > Zhu, and Qian Zhang. > > > Bugs: MESOS-9837 > https://issues.apache.org/jira/browse/MESOS-9837 > > > Repository: mesos > > > Description > --- > > This patch introduces a mechanism for tracking pending futures. > This feature allows detection of hanging operations, which get > stuck on a blocking operation or asynchronously. However, this > feature does not provide any mechanism for tracking pending > promises, because `Promise` objects might not be accessible in > various cases. Thereby, we introduce a new class that can be > used to track pending futures, so it might facilitate debugging > of stuck issues. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/common/future_tracker.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70887/diff/4/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70887/#review216614 --- Fix it, then Ship it! src/common/future_track.hpp Lines 40 (patched) <https://reviews.apache.org/r/70887/#comment303825> Do you need the args to be ordered? If not, probably we could use `unordered_map` (or our stout hashmap wrapper)? the complexity is O(1) in average. src/common/future_track.hpp Lines 44-45 (patched) <https://reviews.apache.org/r/70887/#comment303826> nits: ``` return name == that.name && component == that.component && args == that.args; ``` src/common/future_track.hpp Lines 83 (patched) <https://reviews.apache.org/r/70887/#comment303840> For efficiency, if we are not expecting to erase frequently, we should use `vector` instead of `list`. in this class, the process will erase pending futures frequently? - Gilbert Song On June 19, 2019, 7:49 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70887/ > --- > > (Updated June 19, 2019, 7:49 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng > Zhu, and Qian Zhang. > > > Bugs: MESOS-9837 > https://issues.apache.org/jira/browse/MESOS-9837 > > > Repository: mesos > > > Description > --- > > This patch introduces a mechanism for tracking pending futures. > This feature allows detection of hanging operations, which get > stuck on a blocking operation or asynchronously. However, this > feature does not provide any mechanism for tracking pending > promises, because `Promise` objects might not be accessible in > various cases. Thereby, we introduce a new class that can be > used to track pending futures, so it might facilitate debugging > of stuck issues. > > > Diffs > - > > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/common/future_tracker.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70887/diff/4/ > > > Testing > --- > > > Thanks, > > Andrei Budnik > >
Re: Review Request 71072: Renamed agent flag `--default_shm_size`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71072/#review216639 --- Ship it! Ship It! - Gilbert Song On July 14, 2019, 11:18 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71072/ > --- > > (Updated July 14, 2019, 11:18 p.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9833 > https://issues.apache.org/jira/browse/MESOS-9833 > > > Repository: mesos > > > Description > --- > > Renamed agent flag `--default_shm_size`. > > > Diffs > - > > docs/configuration/agent.md 2046bf25838c4bba9409b0d5e7b29c35882356d2 > docs/isolators/namespaces-ipc.md 43ed8423a7d3877f02408947b78989b79dcd4907 > include/mesos/mesos.proto e0a23913ed64ff6e82c07db227e9d6e0f71a8b55 > include/mesos/v1/mesos.proto af29a140dc7f0a869c8e3b606ec4fb9a22ba1f7b > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 9a984765a317e540e8c7c630dc46cf911f2a33a7 > src/slave/flags.hpp a10bf698dc447b1411c06083c2ba7585d7a78389 > src/slave/flags.cpp b4e3eb99221a09404dbbf813da33607867a78691 > src/tests/containerizer/isolator_tests.cpp > a493a309464f8c7b3cb6f0fc45a2762d12071c67 > > > Diff: https://reviews.apache.org/r/71072/diff/1/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70859: Updated the test `NamespacesIsolatorTest.ROOT_IPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70859/#review216593 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:27 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70859/ > --- > > (Updated July 13, 2019, 1:27 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > This test is updated to verify the backward compatibility is kept > after we implement the configurable IPC namespaces and /dev/shm. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70859/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70860: Added the test `ROOT_IPCNamespaceWithIPCIsolatorDisabled`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70860/#review216594 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:26 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70860/ > --- > > (Updated July 13, 2019, 1:26 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `ROOT_IPCNamespaceWithIPCIsolatorDisabled`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70860/diff/2/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70857: Added the test `ROOT_DisallowShareAgentIPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70857/#review216592 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:22 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70857/ > --- > > (Updated July 13, 2019, 1:22 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `ROOT_DisallowShareAgentIPCNamespace`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70857/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70852: Added the test `NamespacesIsolatorTest.ROOT_ShareAgentIPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70852/#review216591 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:21 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70852/ > --- > > (Updated July 13, 2019, 1:21 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `NamespacesIsolatorTest.ROOT_ShareAgentIPCNamespace`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70852/diff/2/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70849: Added the test `NamespacesIsolatorTest.ROOT_PrivateIPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70849/#review216590 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:19 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70849/ > --- > > (Updated July 13, 2019, 1:19 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `NamespacesIsolatorTest.ROOT_PrivateIPCNamespace`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70849/diff/2/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70845: Added the test `NamespacesIsolatorTest.ROOT_ShareIPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70845/#review216589 --- Ship it! Ship It! - Gilbert Song On July 13, 2019, 1:16 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70845/ > --- > > (Updated July 13, 2019, 1:16 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `NamespacesIsolatorTest.ROOT_ShareIPCNamespace`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > bb2cda47ad1fdd7ad16c419eb04f2e0b9293d2b6 > > > Diff: https://reviews.apache.org/r/70845/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70870: Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70870/#review216564 --- Ship it! Ship It! - Gilbert Song On June 17, 2019, 7:42 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70870/ > --- > > (Updated June 17, 2019, 7:42 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9828 > https://issues.apache.org/jira/browse/MESOS-9828 > > > Repository: mesos > > > Description > --- > > Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm. > > > Diffs > - > > docs/isolators/namespaces-ipc.md e550e752ef0ca4bc363fd3c5c190c44f5dd4187a > > > Diff: https://reviews.apache.org/r/70870/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70845: Added the test `NamespacesIsolatorTest.ROOT_ShareIPCNamespace`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70845/#review216551 --- seems like this test fails consistently? - Gilbert Song On June 12, 2019, 11:57 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70845/ > --- > > (Updated June 12, 2019, 11:57 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added the test `NamespacesIsolatorTest.ROOT_ShareIPCNamespace`. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > 9c14f3acbc19631b2f5cac4dc7cd9caba8527712 > > > Diff: https://reviews.apache.org/r/70845/diff/2/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70820/#review216550 --- Fix it, then Ship it! src/slave/containerizer/mesos/isolators/filesystem/linux.cpp Lines 766 (patched) <https://reviews.apache.org/r/70820/#comment303764> You meant "not enabled"? - Gilbert Song On June 12, 2019, 7:32 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70820/ > --- > > (Updated June 12, 2019, 7:32 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9826 > https://issues.apache.org/jira/browse/MESOS-9826 > > > Repository: mesos > > > Description > --- > > If `namespaces/ipc` isolator is not enabled, for backward > compatibility /dev/shm will still be handled in `filesystem/linux` > isolator as before. Otherwise, both /dev/shm and IPC namespace > will be handled by `namespaces/ipc` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 3cfb6e97a565420c8be2a0e31b481b39cd09d9da > > > Diff: https://reviews.apache.org/r/70820/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70844: Implemented `cleanup` method of the `namespaces/ipc` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70844/#review216549 --- Fix it, then Ship it! src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 397-398 (patched) <https://reviews.apache.org/r/70844/#comment303763> I would add containerID and the dir. - Gilbert Song On June 12, 2019, 7:36 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70844/ > --- > > (Updated June 12, 2019, 7:36 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Implemented `cleanup` method of the `namespaces/ipc` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp > 32c888309ca536d944e4d73641aed214805ccce2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 > > > Diff: https://reviews.apache.org/r/70844/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70798: Improved `namespaces/ipc` isolator for configurable IPC support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70798/#review216548 --- Fix it, then Ship it! src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 190-192 (patched) <https://reviews.apache.org/r/70798/#comment303761> nits: ``` containerConfig.has_rootfs() ? path::join(containerConfig.rootfs(), "/dev/shm") : "/dev/shm", ``` src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 220-222 (patched) <https://reviews.apache.org/r/70798/#comment303762> ditto - Gilbert Song On June 29, 2019, 7:10 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70798/ > --- > > (Updated June 29, 2019, 7:10 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Improved `namespaces/ipc` isolator for configurable IPC support. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp > 32c888309ca536d944e4d73641aed214805ccce2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 > src/slave/containerizer/mesos/paths.hpp > a5e0920bd4d753f6ba6f3b092c92cb405e9edf35 > src/slave/containerizer/mesos/paths.cpp > 4281abc522c942c87fcfd811af26f95cbd6f734f > src/tests/containerizer/isolator_tests.cpp > 9c14f3acbc19631b2f5cac4dc7cd9caba8527712 > > > Diff: https://reviews.apache.org/r/70798/diff/5/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 71048: Fixed the flaky RoleTest.RolesEndpointContainsConsumedQuota test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71048/#review216490 --- Ship it! Ship It! - Gilbert Song On July 10, 2019, 9:53 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71048/ > --- > > (Updated July 10, 2019, 9:53 a.m.) > > > Review request for mesos, Gilbert Song and Meng Zhu. > > > Bugs: MESOS-9886 > https://issues.apache.org/jira/browse/MESOS-9886 > > > Repository: mesos > > > Description > --- > > When this test was run as root in CI, the --launcher gets implicitly > set to `linux` whereas it gets implicitly set to `posix` when run > as non-root. When run as `linux`, the use of cgroups causes the > agents to detect each other's containers as orphans and kill them. > > The fix for now is to force the launcher to be posix. > > > Diffs > - > > src/tests/role_tests.cpp f7a8234030387efbdf7f0353270a9a04d56b164c > > > Diff: https://reviews.apache.org/r/71048/diff/1/ > > > Testing > --- > > Ran in repetition as root. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 70906: Fixed pid checkpointing for `TestContainerizer`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70906/#review216426 --- Ship it! Ship It! - Gilbert Song On July 8, 2019, 2:20 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70906/ > --- > > (Updated July 8, 2019, 2:20 p.m.) > > > Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu. > > > Repository: mesos > > > Description > --- > > In order for a `MockExecutor` to be able to reregister after agent > restart a persisted pid is required. This patch adds checkpointing of > the pid. > > > Diffs > - > > src/tests/containerizer.cpp fab7e81339f3b7b3fec2a4f6cb7e3a52df2607ed > > > Diff: https://reviews.apache.org/r/70906/diff/3/ > > > Testing > --- > > `make check` > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 71005: Used euid to determine subprocess' user when launching tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71005/#review216406 --- Ship it! Ship It! - Gilbert Song On July 3, 2019, 4:12 a.m., fei long wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71005/ > --- > > (Updated July 3, 2019, 4:12 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: mesos-9876 > https://issues.apache.org/jira/browse/mesos-9876 > > > Repository: mesos > > > Description > --- > > Used euid to determine subprocess' user when launching tasks. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp > b29ec556399a40aa662987a11a2b64e6a16de889 > > > Diff: https://reviews.apache.org/r/71005/diff/1/ > > > Testing > --- > > All testing cases passed. > > > Thanks, > > fei long > >
Re: Review Request 70989: Set the `MESOS_ALLOCATION_ROLE` environment variable for task.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70989/#review216328 --- Ship it! Ship It! - Gilbert Song On July 2, 2019, 7:38 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70989/ > --- > > (Updated July 2, 2019, 7:38 a.m.) > > > Review request for mesos and Gilbert Song. > > > Bugs: MESOS-9874 > https://issues.apache.org/jira/browse/MESOS-9874 > > > Repository: mesos > > > Description > --- > > Set the `MESOS_ALLOCATION_ROLE` environment variable for task. > > > Diffs > - > > src/docker/docker.cpp 9ebb5f27464b898895e03971fa462349fb68e1f0 > src/launcher/default_executor.cpp 5837cfa4deba557cae43112092ff24b97137951f > src/launcher/executor.cpp 38d82614ed82e8a6644334f0401cecdee6a025bf > > > Diff: https://reviews.apache.org/r/70989/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70948: Attempting to get Jenkins pipeline build expiry fixed.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70948/#review216146 --- Ship it! Ship It! - Gilbert Song On June 25, 2019, 5:16 p.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70948/ > --- > > (Updated June 25, 2019, 5:16 p.m.) > > > Review request for mesos, Benjamin Bannier, Gilbert Song, and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Attempting to get Jenkins pipeline build expiry fixed. > > > Diffs > - > > support/jenkins/Jenkinsfile-packaging-centos > aa8f37f6cd661e767e55ca75095a91827ebedb21 > > > Diff: https://reviews.apache.org/r/70948/diff/2/ > > > Testing > --- > > none - cannot test this without committing it right now > > info for the option in use is taken from: > https://stackoverflow.com/a/53141684/91282 > > > Thanks, > > Till Toenshoff > >
Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.
> On June 20, 2019, 5:27 p.m., Gilbert Song wrote: > > could we consider to do the decouple now? probably still need a deprecation cycle :( - Gilbert --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70820/#review216030 --- On June 12, 2019, 7:32 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70820/ > --- > > (Updated June 12, 2019, 7:32 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9826 > https://issues.apache.org/jira/browse/MESOS-9826 > > > Repository: mesos > > > Description > --- > > If `namespaces/ipc` isolator is not enabled, for backward > compatibility /dev/shm will still be handled in `filesystem/linux` > isolator as before. Otherwise, both /dev/shm and IPC namespace > will be handled by `namespaces/ipc` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 3cfb6e97a565420c8be2a0e31b481b39cd09d9da > > > Diff: https://reviews.apache.org/r/70820/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70820/#review216030 --- could we consider to do the decouple now? - Gilbert Song On June 12, 2019, 7:32 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70820/ > --- > > (Updated June 12, 2019, 7:32 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9826 > https://issues.apache.org/jira/browse/MESOS-9826 > > > Repository: mesos > > > Description > --- > > If `namespaces/ipc` isolator is not enabled, for backward > compatibility /dev/shm will still be handled in `filesystem/linux` > isolator as before. Otherwise, both /dev/shm and IPC namespace > will be handled by `namespaces/ipc` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 3cfb6e97a565420c8be2a0e31b481b39cd09d9da > > > Diff: https://reviews.apache.org/r/70820/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70844: Implemented `cleanup` method of the `namespaces/ipc` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70844/#review216029 --- src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 400 (patched) <https://reviews.apache.org/r/70844/#comment302992> if the shmPath not exists, should we log a warning? - Gilbert Song On June 12, 2019, 7:36 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70844/ > --- > > (Updated June 12, 2019, 7:36 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Implemented `cleanup` method of the `namespaces/ipc` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp > 32c888309ca536d944e4d73641aed214805ccce2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 > > > Diff: https://reviews.apache.org/r/70844/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70798: Improved `namespaces/ipc` isolator for configurable IPC support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70798/#review216026 --- src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 52 (patched) <https://reviews.apache.org/r/70798/#comment302982> consider to move it to containerizer/mesos/path.hpp? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 52-54 (patched) <https://reviews.apache.org/r/70798/#comment302987> consider to return Result for no parent case? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 56 (patched) <https://reviews.apache.org/r/70798/#comment302986> a bit confusing. the parentId could be assigned as a containerId without a parent? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 90-93 (patched) <https://reviews.apache.org/r/70798/#comment302985> what if for SHARE_PARENT case but the container does not have a parent? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 137-143 (patched) <https://reviews.apache.org/r/70798/#comment302988> is this a new dependency? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Lines 212 (patched) <https://reviews.apache.org/r/70798/#comment302989> are we going to remove the `createContainerMount()` in linux filesystem isolator? src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp Line 94 (original), 382 (patched) <https://reviews.apache.org/r/70798/#comment302990> if we introduce cleanup(), does it mean we could avoid the dependency on linux filesystem isolator? src/slave/containerizer/mesos/paths.hpp Lines 85 (patched) <https://reviews.apache.org/r/70798/#comment302978> how about `CONTAINER_SHM_DIRECTORY` or just `CONTAINER_SHM`? - Gilbert Song On June 11, 2019, 7:33 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70798/ > --- > > (Updated June 11, 2019, 7:33 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Improved `namespaces/ipc` isolator for configurable IPC support. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp > 32c888309ca536d944e4d73641aed214805ccce2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp > 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 > src/slave/containerizer/mesos/paths.hpp > a5e0920bd4d753f6ba6f3b092c92cb405e9edf35 > src/slave/containerizer/mesos/paths.cpp > 4281abc522c942c87fcfd811af26f95cbd6f734f > src/tests/containerizer/isolator_tests.cpp > 9c14f3acbc19631b2f5cac4dc7cd9caba8527712 > > > Diff: https://reviews.apache.org/r/70798/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70775: Added `ipc_mode` and `shm_size` fields into `LinuxInfo`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70775/#review215974 --- Fix it, then Ship it! include/mesos/mesos.proto Lines 3269-3270 (patched) <https://reviews.apache.org/r/70775/#comment302899> delete: with a possibility to share them with its child containers Is a nested container allowed to have its own ipc namespace and /dev/shm? include/mesos/mesos.proto Lines 3273 (patched) <https://reviews.apache.org/r/70775/#comment302893> s/with/from/g include/mesos/mesos.proto Lines 3275 (patched) <https://reviews.apache.org/r/70775/#comment302894> s/with/from/g include/mesos/mesos.proto Lines 3277 (patched) <https://reviews.apache.org/r/70775/#comment302895> ditto include/mesos/mesos.proto Lines 3278 (patched) <https://reviews.apache.org/r/70775/#comment302896> ditto include/mesos/mesos.proto Lines 3279 (patched) <https://reviews.apache.org/r/70775/#comment302897> s/do it/ share from the agent's/g include/mesos/mesos.proto Lines 3289 (patched) <https://reviews.apache.org/r/70775/#comment302907> period `.` at the end. include/mesos/mesos.proto Lines 3290-3294 (patched) <https://reviews.apache.org/r/70775/#comment302908> Can we split into different sentences to make it more clear? include/mesos/mesos.proto Lines 3296 (patched) <https://reviews.apache.org/r/70775/#comment302909> s/handling/support/g include/mesos/mesos.proto Lines 3297 (patched) <https://reviews.apache.org/r/70775/#comment302910> s/,/./g include/mesos/mesos.proto Lines 3299-3301 (patched) <https://reviews.apache.org/r/70775/#comment302911> add `()` to i.e.,... include/mesos/v1/mesos.proto Lines 3259 (patched) <https://reviews.apache.org/r/70775/#comment302912> ditto - Gilbert Song On June 6, 2019, 4:23 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70775/ > --- > > (Updated June 6, 2019, 4:23 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9827 > https://issues.apache.org/jira/browse/MESOS-9827 > > > Repository: mesos > > > Description > --- > > Added `ipc_mode` and `shm_size` fields into `LinuxInfo`. > > > Diffs > - > > include/mesos/mesos.proto dc6a87f14864cb4d46b9856f251c2946a475162d > include/mesos/v1/mesos.proto e8086e0f70cac73876a8ae31db3365b5059b5c44 > > > Diff: https://reviews.apache.org/r/70775/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70774: Added `--default_shm_size` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70774/#review215972 --- Ship it! Ship It! - Gilbert Song On June 18, 2019, 11:26 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70774/ > --- > > (Updated June 18, 2019, 11:26 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9833 > https://issues.apache.org/jira/browse/MESOS-9833 > > > Repository: mesos > > > Description > --- > > Added `--default_shm_size` agent flag. > > > Diffs > - > > docs/configuration/agent.md 91a5e931c35fb353c87bee35004cb73e8546c8d3 > src/slave/flags.hpp 9684a09071be930f25a7d10821ee65b5a965fa83 > src/slave/flags.cpp e23061ad929c8e3c24806f52c95f82766e3d1911 > > > Diff: https://reviews.apache.org/r/70774/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70773: Added `--disallow_sharing_agent_ipc_namespace` agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70773/#review215970 --- Ship it! Ship It! - Gilbert Song On June 18, 2019, 11:21 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70773/ > --- > > (Updated June 18, 2019, 11:21 p.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9788 > https://issues.apache.org/jira/browse/MESOS-9788 > > > Repository: mesos > > > Description > --- > > Added `--disallow_sharing_agent_ipc_namespace` agent flag. > > > Diffs > - > > docs/configuration/agent.md 91a5e931c35fb353c87bee35004cb73e8546c8d3 > src/slave/flags.hpp 9684a09071be930f25a7d10821ee65b5a965fa83 > src/slave/flags.cpp e23061ad929c8e3c24806f52c95f82766e3d1911 > > > Diff: https://reviews.apache.org/r/70773/diff/3/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70740: Windows: Removed multiple categories of sources from the build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70740/#review215951 --- Ship it! Ship It! - Gilbert Song On May 28, 2019, 4:11 p.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70740/ > --- > > (Updated May 28, 2019, 4:11 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Gilbert Song. > > > Repository: mesos > > > Description > --- > > This removes some categories of sources from the Windows build, > where it is possible to do so with minimal ifdef-ing. > > The features removed are all Linux-specific features that cannot be > feasibly ported to Windows, including: > * Container Storage Interface (CSI) > * Docker image provisioning, specifically related to V2 > * Open Container Interface > * Volume GID Manager > > Protobufs are excluded where possible, but many of the above categories > of protobufs are interleaved with other protobufs or source code, > which makes exclusion non-trivial. For example, CSI V0 protobufs > cannot be excluded without a large change; or libseccomp is a Linux-only > feature, but its protobufs are now required to build the Mesos > containerizer's protobufs. > > Docker image provisioning was semi-trivial to exclude, because the > related components (provisioner & URI fetcher) are somewhat modularized. > > > Diffs > - > > src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 > src/slave/containerizer/mesos/containerizer.cpp > 043244841a73fa3f5f7119bc38f6d3a04be8990b > src/slave/containerizer/mesos/provisioner/store.cpp > 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a > src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 > src/tests/uri_fetcher_tests.cpp c727cc52e82a396fe187a00c8cc3c9e78a919c5d > src/uri/fetcher.hpp cc4bd93b3b8bcb7803f8f912f4ad9d3cf45a58a9 > src/uri/fetcher.cpp 8db43eb9763f1cf8040db93a1f03aae0fe9ab3c7 > > > Diff: https://reviews.apache.org/r/70740/diff/1/ > > > Testing > --- > > cmake --build . --target check > > This slightly decreases the memory footprint of the build, and allowed my > build instance (4GB mem) to proceed beyond some agent files (which is where > the Windows CI is also running out of memory). It still ran out of memory > when compiling tests however. After giving the instance more memory (8GB), > the build succeeds. > > > Thanks, > > Joseph Wu > >
Re: Review Request 70740: Windows: Removed multiple categories of sources from the build.
> On June 18, 2019, 4:09 p.m., Gilbert Song wrote: > > src/uri/fetcher.hpp > > Lines 43-44 (original), 46-49 (patched) > > <https://reviews.apache.org/r/70740/diff/1/?file=2146973#file2146973line47> > > > > Probably we still want to keep the docker fetche plugin for windows. > > Otherwise, it is a partial revert for > > https://issues.apache.org/jira/browse/MESOS-9159 > > > > Mesos on Windows does not support using a docker image yet, fetching an > > image is supported. We have three unit tests to verify the work: > > ``` > > DISABLED_INTERNET_CURL_FetchManifest > > DISABLED_INTERNET_CURL_FetchImage > > DISABLED_INTERNET_CURL_InvokeFetchByName > > ``` > > > > They were disabled recently due to flakiness. Is the change for URI > > fetcher just for short term? We can revisit this later. - Gilbert --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70740/#review215949 --- On May 28, 2019, 4:11 p.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70740/ > ------- > > (Updated May 28, 2019, 4:11 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Gilbert Song. > > > Repository: mesos > > > Description > --- > > This removes some categories of sources from the Windows build, > where it is possible to do so with minimal ifdef-ing. > > The features removed are all Linux-specific features that cannot be > feasibly ported to Windows, including: > * Container Storage Interface (CSI) > * Docker image provisioning, specifically related to V2 > * Open Container Interface > * Volume GID Manager > > Protobufs are excluded where possible, but many of the above categories > of protobufs are interleaved with other protobufs or source code, > which makes exclusion non-trivial. For example, CSI V0 protobufs > cannot be excluded without a large change; or libseccomp is a Linux-only > feature, but its protobufs are now required to build the Mesos > containerizer's protobufs. > > Docker image provisioning was semi-trivial to exclude, because the > related components (provisioner & URI fetcher) are somewhat modularized. > > > Diffs > - > > src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 > src/slave/containerizer/mesos/containerizer.cpp > 043244841a73fa3f5f7119bc38f6d3a04be8990b > src/slave/containerizer/mesos/provisioner/store.cpp > 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a > src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 > src/tests/uri_fetcher_tests.cpp c727cc52e82a396fe187a00c8cc3c9e78a919c5d > src/uri/fetcher.hpp cc4bd93b3b8bcb7803f8f912f4ad9d3cf45a58a9 > src/uri/fetcher.cpp 8db43eb9763f1cf8040db93a1f03aae0fe9ab3c7 > > > Diff: https://reviews.apache.org/r/70740/diff/1/ > > > Testing > --- > > cmake --build . --target check > > This slightly decreases the memory footprint of the build, and allowed my > build instance (4GB mem) to proceed beyond some agent files (which is where > the Windows CI is also running out of memory). It still ran out of memory > when compiling tests however. After giving the instance more memory (8GB), > the build succeeds. > > > Thanks, > > Joseph Wu > >
Re: Review Request 70740: Windows: Removed multiple categories of sources from the build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70740/#review215949 --- src/uri/fetcher.hpp Lines 43-44 (original), 46-49 (patched) <https://reviews.apache.org/r/70740/#comment302880> Probably we still want to keep the docker fetche plugin for windows. Otherwise, it is a partial revert for https://issues.apache.org/jira/browse/MESOS-9159 Mesos on Windows does not support using a docker image yet, fetching an image is supported. We have three unit tests to verify the work: ``` DISABLED_INTERNET_CURL_FetchManifest DISABLED_INTERNET_CURL_FetchImage DISABLED_INTERNET_CURL_InvokeFetchByName ``` They were disabled recently due to flakiness. Is the change for URI fetcher just for short term? - Gilbert Song On May 28, 2019, 4:11 p.m., Joseph Wu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70740/ > --- > > (Updated May 28, 2019, 4:11 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Gilbert Song. > > > Repository: mesos > > > Description > --- > > This removes some categories of sources from the Windows build, > where it is possible to do so with minimal ifdef-ing. > > The features removed are all Linux-specific features that cannot be > feasibly ported to Windows, including: > * Container Storage Interface (CSI) > * Docker image provisioning, specifically related to V2 > * Open Container Interface > * Volume GID Manager > > Protobufs are excluded where possible, but many of the above categories > of protobufs are interleaved with other protobufs or source code, > which makes exclusion non-trivial. For example, CSI V0 protobufs > cannot be excluded without a large change; or libseccomp is a Linux-only > feature, but its protobufs are now required to build the Mesos > containerizer's protobufs. > > Docker image provisioning was semi-trivial to exclude, because the > related components (provisioner & URI fetcher) are somewhat modularized. > > > Diffs > - > > src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 > src/slave/containerizer/mesos/containerizer.cpp > 043244841a73fa3f5f7119bc38f6d3a04be8990b > src/slave/containerizer/mesos/provisioner/store.cpp > 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a > src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 > src/tests/uri_fetcher_tests.cpp c727cc52e82a396fe187a00c8cc3c9e78a919c5d > src/uri/fetcher.hpp cc4bd93b3b8bcb7803f8f912f4ad9d3cf45a58a9 > src/uri/fetcher.cpp 8db43eb9763f1cf8040db93a1f03aae0fe9ab3c7 > > > Diff: https://reviews.apache.org/r/70740/diff/1/ > > > Testing > --- > > cmake --build . --target check > > This slightly decreases the memory footprint of the build, and allowed my > build instance (4GB mem) to proceed beyond some agent files (which is where > the Windows CI is also running out of memory). It still ran out of memory > when compiling tests however. After giving the instance more memory (8GB), > the build succeeds. > > > Thanks, > > Joseph Wu > >
Re: Review Request 70863: Assign cgroup processes after configuring the subsystem.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70863/#review215947
---
Fix it, then Ship it!
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 665-666 (original), 665-666 (patched)
<https://reviews.apache.org/r/70863/#comment302878>
Tech debt, I would suggest to put this part of codes here.
```
// We currently can't call `subsystem->isolate()` on nested
// containers, because we don't call `prepare()`, `recover()`, or
// `cleanup()` on them either. If we were to call `isolate()` on
// them, the call would likely fail because the subsystem doesn't
// know about the container. This is currently OK because the only
// cgroup isolator that even implements `isolate()` is the
// `NetClsSubsystem` and it doesn't do anything with the `pid`
// passed in.
//
// TODO(klueska): In the future we should revisit this to make
// sure that doing things this way is sufficient (or otherwise
// update our invariants to allow us to call this here).
if (containerId.has_parent()) {
return Nothing();
}
```
- Gilbert Song
On June 16, 2019, 7:47 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70863/
> ---
>
> (Updated June 16, 2019, 7:47 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
>
>
> Bugs: MESOS-9805
> https://issues.apache.org/jira/browse/MESOS-9805
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Currently, the PID targeted by the cgroups isolator is moved into
> the cgroup before the subsystem runs to apply any type-specific
> cgroup configuration. We should reverse the order of this so that
> the PID is only moved once the cgroup is fully configured by the
> subsystem.
>
> A specific case that can happen is where a PID is assigned to a net_cls
> cgroup before that cgroup has its class ID set. This intermediate
> process state can be observed by system monitoring process, causing
> confusion that is hard to debug.
>
>
> Diffs
> -
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp
> 4a1871b3b06b54a02dfe09289f7fb304a3f7f24c
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
> e7819d732172bdbd215106e3b781588c1f78b2ec
>
>
> Diff: https://reviews.apache.org/r/70863/diff/1/
>
>
> Testing
> ---
>
> sudo make check (Fedora 30)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 70862: Update `EXPECT` to `ASSERT` in blkio tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70862/#review215940 --- Ship it! Ship It! - Gilbert Song On June 16, 2019, 7:13 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70862/ > --- > > (Updated June 16, 2019, 7:13 p.m.) > > > Review request for mesos, Gilbert Song and Qian Zhang. > > > Repository: mesos > > > Description > --- > > Since the blkio cgroup tests immediately require the result of > `blkio_statistics()`, we should test for the presence of statistics > with `ASSERT_TRUE` rather than `EXPECT_TRUE`. > > > Diffs > - > > src/tests/containerizer/cgroups_isolator_tests.cpp > eef8eb28ea57ef0a2a3c626ac9aee202eb7231d9 > > > Diff: https://reviews.apache.org/r/70862/diff/1/ > > > Testing > --- > > make check (Fedora 30) > > > Thanks, > > James Peach > >
Re: Review Request 70827: Improved container-specific cgroups test by checking `cpu.shares`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70827/#review215823 --- Ship it! Ship It! - Gilbert Song On June 10, 2019, 7:48 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70827/ > --- > > (Updated June 10, 2019, 7:48 p.m.) > > > Review request for mesos, Andrei Budnik, Gilbert Song, and James Peach. > > > Bugs: MESOS-9769 > https://issues.apache.org/jira/browse/MESOS-9769 > > > Repository: mesos > > > Description > --- > > This is to ensure the symbolic links (see below as an example) we > create for the container exist. > ln -s /sys/fs/cgroup/cpu,cpuacct /sys/fs/cgroup/cpu > > > Diffs > - > > src/tests/containerizer/cgroups_isolator_tests.cpp > 957f72d78f9ab0bf2775687915099c0109dac6e1 > > > Diff: https://reviews.apache.org/r/70827/diff/1/ > > > Testing > --- > > sudo make check > > The tests updated in this patch would fail without the previous patch. > > > Thanks, > > Qian Zhang > >
Re: Review Request 70826: Supported file operations for command tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70826/#review215818 --- Ship it! Ship It! - Gilbert Song On June 10, 2019, 7:46 p.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70826/ > --- > > (Updated June 10, 2019, 7:46 p.m.) > > > Review request for mesos, Andrei Budnik, Gilbert Song, and James Peach. > > > Bugs: MESOS-9769 > https://issues.apache.org/jira/browse/MESOS-9769 > > > Repository: mesos > > > Description > --- > > Supported file operations for command tasks. > > > Diffs > - > > src/launcher/executor.cpp fa4bcaad9ac36bf380484dadb14d0b0a86a30aae > > > Diff: https://reviews.apache.org/r/70826/diff/1/ > > > Testing > --- > > > Thanks, > > Qian Zhang > >
Re: Review Request 70811: Fixed compilation error on Mac OS.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70811/#review215755 --- Ship it! Ship It! - Gilbert Song On June 7, 2019, 10:21 a.m., Andrei Budnik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70811/ > --- > > (Updated June 7, 2019, 10:21 a.m.) > > > Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and > Jiang Yan Xu. > > > Repository: mesos > > > Description > --- > > This patch adds missing switch case to fix compilation error introduced > in `bc5a57122635`. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp > a69a68823842171bb15ddf34a504e5ce4af232b0 > > > Diff: https://reviews.apache.org/r/70811/diff/1/ > > > Testing > --- > > internal CI > > > Thanks, > > Andrei Budnik > >
