Review Request 52254: Updated aufs mount with `rw` and `ro+wh` options.

[Qian Zhang]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Updated aufs mount with `rw` and `ro+wh` options.


Diffs
-

  src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
c69dc425987661cd84ac882b2dc90cc8d7ca1d45 

Diff: https://reviews.apache.org/r/52254/diff/


Testing
---

make check on Ubuntu 14.04.


Thanks,

Qian Zhang

Re: Review Request 52254: Updated aufs mount with `rw` and `ro+wh` options.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 26, 2016, 12:26 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52254/
> ---
> 
> (Updated Sept. 26, 2016, 12:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6002
> https://issues.apache.org/jira/browse/MESOS-6002
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated aufs mount with `rw` and `ro+wh` options.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> c69dc425987661cd84ac882b2dc90cc8d7ca1d45 
> 
> Diff: https://reviews.apache.org/r/52254/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush

[Anastasia Braginsky]

> On Sept. 25, 2016, 9:13 p.m., Michael Stack wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java,
> >  line 93
> > 
> >
> > The hyphen will look weird when the javadoc is generated just FYI
> > 
> > Suffix? Is that the end of the pipeline?

Deleted the hyphen. And indeed, the name "suffix" is because we swap out only 
the end of the pipeline.


> On Sept. 25, 2016, 9:13 p.m., Michael Stack wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java,
> >  line 135
> > 
> >
> > Why do it in two places? This seems right place to do it... not in 
> > constructor.

Actually in coordination with comment from Anoop, I removed the initiation of 
the action here and left only in constructor. The initiation of the action in 
the start of the compaction is needed only for tests and there is no need to 
perform this initiation each time the compaction starts in the production code.


- Anastasia


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


On Sept. 22, 2016, 9:12 a.m., Anastasia Braginsky wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51785/
> ---
> 
> (Updated Sept. 22, 2016, 9:12 a.m.)
> 
> 
> Review request for hbase.
> 
> 
> Repository: hbase-git
> 
> 
> Description
> ---
> 
> This is a step toward final compacting memstore that allowes two modes of 
> work: index-compaction and data-compaction. 
> 
> The index-compaction means that when the new segment is pushed into the 
> pipeline, it is flattened and probably merged with old segments in the 
> pipeline. The new merge "feature" induces no data-copy-compaction and no 
> speculative SQM scan. 
> The compacting memstore of the data-compaction type means the usage of the 
> data-copy-compaction.
> 
> 
> Diffs
> -
> 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java
>  177f222 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java
>  6a13f43 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java
>  3ca4b0c 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java
>  12b7916 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java
>  714ffe3 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorIterator.java
>  2eafb42 
>   
> hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java
>  510ebbd 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingMemStore.java
>  211a6d8 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingToCellArrayMapMemStore.java
>  fefe2c1 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPerColumnFamilyFlush.java
>  6bfaa59 
>   
> hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWalAndCompactingMemStoreFlush.java
>  74826b0 
> 
> Diff: https://reviews.apache.org/r/51785/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anastasia Braginsky
> 
>

Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Michael Park.


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


Repository: mesos


Description
---

Fixed an assertion failure when testing for systemd support.


Diffs
-

  src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 

Diff: https://reviews.apache.org/r/52258/diff/


Testing
---

See description.


Thanks,

Jan Schlicht

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Jan Schlicht]

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

(Updated Sept. 26, 2016, 12:18 p.m.)


Review request for mesos, Benjamin Bannier and Michael Park.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 

Diff: https://reviews.apache.org/r/52258/diff/


Testing (updated)
---

make check


Thanks,

Jan Schlicht

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Sept. 26, 2016, 10:18 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52258/
> ---
> 
> (Updated Sept. 26, 2016, 10:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6248
> https://issues.apache.org/jira/browse/MESOS-6248
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 
> 
> Diff: https://reviews.apache.org/r/52258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Benjamin Bannier]

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




src/linux/systemd.cpp (line 202)


Let's `s/"No such entry"/"No such file or directory"/` or even `"No such 
file"` here.


- Benjamin Bannier


On Sept. 26, 2016, 12:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52258/
> ---
> 
> (Updated Sept. 26, 2016, 12:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6248
> https://issues.apache.org/jira/browse/MESOS-6248
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 
> 
> Diff: https://reviews.apache.org/r/52258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Jan Schlicht]

> On Sept. 26, 2016, 12:38 p.m., Benjamin Bannier wrote:
> > src/linux/systemd.cpp, line 202
> > 
> >
> > Let's `s/"No such entry"/"No such file or directory"/` or even `"No 
> > such file"` here.

Okay, changing it to "file does not exist".


- Jan


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


On Sept. 26, 2016, 12:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52258/
> ---
> 
> (Updated Sept. 26, 2016, 12:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6248
> https://issues.apache.org/jira/browse/MESOS-6248
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 
> 
> Diff: https://reviews.apache.org/r/52258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Jan Schlicht]

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

(Updated Sept. 26, 2016, 1:49 p.m.)


Review request for mesos, Benjamin Bannier and Michael Park.


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


Repository: mesos


Description (updated)
---

Fixed an assertion failure when testing for systemd support.


Diffs (updated)
-

  src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 

Diff: https://reviews.apache.org/r/52258/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Jan Schlicht]

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

(Updated Sept. 26, 2016, 1:49 p.m.)


Review request for mesos, Benjamin Bannier and Michael Park.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 

Diff: https://reviews.apache.org/r/52258/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Sept. 26, 2016, 11:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52258/
> ---
> 
> (Updated Sept. 26, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6248
> https://issues.apache.org/jira/browse/MESOS-6248
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 
> 
> Diff: https://reviews.apache.org/r/52258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 52258: Fixed an assertion failure when testing for systemd support.

[Michael Park]

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



Submitted with `"does not exist"` rather than `"file does not exist"` for 
consistency with: 
https://github.com/apache/mesos/blob/1.0.1/src/linux/systemd.cpp#L183-L186

- Michael Park


On Sept. 26, 2016, 11:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52258/
> ---
> 
> (Updated Sept. 26, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6248
> https://issues.apache.org/jira/browse/MESOS-6248
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.cpp 96813b7b7411668782194d21324aaef049e8c1a7 
> 
> Diff: https://reviews.apache.org/r/52258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 51257: Add external process container logger.

[Will Rouesnel]

> On Sept. 7, 2016, 7:34 p.m., Joseph Wu wrote:
> > src/slave/container_loggers/lib_externallogger.hpp, lines 52-61
> > 
> >
> > This flag should be required.  You can use `flags.add` like:
> > ```
> > add(&external_logger_cmd,
> > "external_logger_cmd",
> > None(),
> > "Path to the external command which will read STDIN for logs",
> > [](const std::string& executablePath) -> Option {
> >   if (!os::exists(executablePath)) {
> >   return Error("Cannot find: " + executablePath);
> >   }
> >   return None();
> > });
> > ```
> > 
> > The `None()` as the third argument is an alias.  We need this to 
> > disambiguate the various overloads of `flags.add`.
> > 
> > ---
> > 
> > Also, add a newline after this block.
> 
> Will Rouesnel wrote:
> This doesn't actually compile for me at the moment, unless something's 
> changed on master I'm not rebased against?

Ah, looks like the sample was missing an extra nullptr to for the default value 
disambiguation.


- Will


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


On Sept. 25, 2016, 6:11 p.m., Will Rouesnel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51257/
> ---
> 
> (Updated Sept. 25, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6003
> https://issues.apache.org/jira/browse/MESOS-6003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the external process container logger. This functions like the
> logrotate container logger, but instead calls a custom host binary
> (or script) and passes the executorInfo as JSON via environment
> variables. This makes it very easy for users to configure custom
> logging solutions, without needing to write and maintain logging
> modules.
> 
> Tests passing and complete.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 410b5f2e5bc50a5aa7856645c8cf4a3e43505a84 
>   src/slave/container_loggers/lib_externallogger.hpp PRE-CREATION 
>   src/slave/container_loggers/lib_externallogger.cpp PRE-CREATION 
>   src/tests/container_logger_external.sh PRE-CREATION 
>   src/tests/container_logger_tests.cpp 
> f76117230e0517ddc3cb8e0bf482085fad6950d2 
>   src/tests/module.hpp e661d95fa44fc1aedfe83c564c826d5b7d32c85b 
>   src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
> 
> Diff: https://reviews.apache.org/r/51257/diff/
> 
> 
> Testing
> ---
> 
> Adds ContainerLoggerTest.EXTERNAL_RecieveEnvironment which tests all major 
> parameters of the change.
> 
> A synthetic external container logger is provided by the script 
> tests/container_logger_external.sh which is setup to fail if any important 
> output is unavailable to the logging process. 
> 
> The other basic checks are duplicated from the Logrotate container logger, 
> from where this change inherits a lot of its code.
> 
> 
> Thanks,
> 
> Will Rouesnel
> 
>

Re: Review Request 52254: Updated aufs mount with `rw` and `ro+wh` options.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52254]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 26, 2016, 7:26 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52254/
> ---
> 
> (Updated Sept. 26, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6002
> https://issues.apache.org/jira/browse/MESOS-6002
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated aufs mount with `rw` and `ro+wh` options.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> c69dc425987661cd84ac882b2dc90cc8d7ca1d45 
> 
> Diff: https://reviews.apache.org/r/52254/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush

[Anastasia Braginsky]

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

(Updated Sept. 26, 2016, 3:27 p.m.)


Review request for hbase.


Changes
---

The recent update is splitting MemStoreCompactorIterator into two classes: 
MemStoreCompactorSegmentsIterator and MemStoreMergerSegmentsIterator, both 
doing iterations over list of segments (first with SQM and second without). The 
common between two classes is held in the abstract class 
MemStoreSegmentsIterator, which both new classes extend.


Repository: hbase-git


Description
---

This is a step toward final compacting memstore that allowes two modes of work: 
index-compaction and data-compaction. 

The index-compaction means that when the new segment is pushed into the 
pipeline, it is flattened and probably merged with old segments in the 
pipeline. The new merge "feature" induces no data-copy-compaction and no 
speculative SQM scan. 
The compacting memstore of the data-compaction type means the usage of the 
data-copy-compaction.


Diffs (updated)
-

  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java
 177f222 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java
 6a13f43 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java
 3ca4b0c 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java
 12b7916 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java
 714ffe3 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorIterator.java
 2eafb42 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorSegmentsIterator.java
 PRE-CREATION 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreMergerSegmentsIterator.java
 PRE-CREATION 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSegmentsIterator.java
 PRE-CREATION 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java
 510ebbd 
  
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/VersionedSegmentsList.java
 2e8bead 
  
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingMemStore.java
 211a6d8 
  
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingToCellArrayMapMemStore.java
 fefe2c1 
  
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestPerColumnFamilyFlush.java
 6bfaa59 
  
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWalAndCompactingMemStoreFlush.java
 74826b0 

Diff: https://reviews.apache.org/r/51785/diff/


Testing
---


Thanks,

Anastasia Braginsky

Re: Review Request 52147: Added support for launching child containers to the default executor.

[Vinod Kone]

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


Fix it, then Ship it!





src/launcher/default_executor.cpp (line 241)


I would change these to LOG(ERROR) because dropping a task group launch is 
an important enough event that we want to surface without expecting users to 
set GLOG_v.

here and everywhere else.


- Vinod Kone


On Sept. 25, 2016, 9:36 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52147/
> ---
> 
> (Updated Sept. 25, 2016, 9:36 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for sending the `LAUNCH_NESTED_CONTAINER`
> call to the agent to launch a child container.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52147/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 52256: Made `FlagsBase::extract` publicly accessible.

[Benjamin Bannier]

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

Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

This function contains important business logic on how environment
variables are interpreted (keys are largely case-insensitive, and
duplicates are dropped in a certain order). Make this function
`public` so users interested in seeing what environment variables
`Flags` would see are not tempted to reimplement it (likely
incompletely or incorrectly).


Diffs
-

  3rdparty/stout/include/stout/flags/flags.hpp 
4ca6c69aeb1e3343e9b0ae2562c450c4c645890c 

Diff: https://reviews.apache.org/r/52256/diff/


Testing
---

Tested on various platforms in internal CI as part of 
https://reviews.apache.org/r/52154/.


Thanks,

Benjamin Bannier

Re: Review Request 52154: Avoided modifying process environment.

[Benjamin Bannier]

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

(Updated Sept. 26, 2016, 6:16 p.m.)


Review request for mesos, Joris Van Remoortere and Till Toenshoff.


Changes
---

Fixed to correctly reproduce overriding behavior of old implementation when 
variables with different case were found, e.g., `SSL_foo` and 
`LIBPROCESS_SSL_Foo` would attempt to update the same variable `foo` and should 
contain compatible values, or `SSL_foo` would likely be overriden by `SSL_Foo` 
(at least in the `C` locale), which would e.g., need to be compatible with 
`LIBPROCESS_SSL_FOO`.


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


Repository: mesos


Description
---

This code modified the process environment in order to support upgrading
on the fly from old-style libprocess SSL variables `SSL_` to
`LIBPROCESS_SSL_`. Modifying the process environment at this point is
not safe as other actors might concurrently read out that same
environment.

Instead avoid changing the process environment altogether since flags
can just as well be read from a map.


Diffs (updated)
-

  3rdparty/libprocess/src/openssl.cpp c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 

Diff: https://reviews.apache.org/r/52154/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

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

[Jacob Janco]

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

(Updated Sept. 26, 2016, 4:27 p.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus Ma, 
and Jiang Yan Xu.


Changes
---

Cleaned up code with Yan's help/suggestions:
- overloaded `allocate()` dispatches `run()`
- an allocation `run()` synchronously runs `_allocate()` and `_deallocate()`


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


Repository: mesos


Description
---

- Triggered allocations dispatch allocate() only
  if there is no pending allocation in the queue.
- Allocation candidates are accumulated and only
  cleared when enqueued allocations are processed.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2c31471ee0f5d6836393bf87ff9ecfd8df835013 
  src/master/allocator/mesos/hierarchical.cpp 
3f51f4194c1ba7c1e4f08c3dd623281ca5754d39 

Diff: https://reviews.apache.org/r/51027/diff/


Testing
---

make check

note: check without filters depends on https://reviews.apache.org/r/51028

With new benchmark https://reviews.apache.org/r/49617: 
Sample output without 51027:
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
Using 1 agents and 3000 frameworks
Added 3000 frameworks in 57251us
Added 1 agents in 3.2134535333mins
allocator settled after  1.6123603833mins
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
 (290578 ms)

Sample output with 51027:
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
Using 1 agents and 3000 frameworks
Added 3000 frameworks in 39817us
Added 1 agents in 3.2286054167mins
allocator settled after  25.525654secs
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
 (220137 ms)


Thanks,

Jacob Janco

Re: Review Request 52147: Added support for launching child containers to the default executor.

[Vinod Kone]

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




src/launcher/default_executor.cpp (line 380)


log here as well?



src/launcher/default_executor.cpp (line 388)


log here as well.

please make a sweep in this file and ensure there are no silent returns for 
important events.


- Vinod Kone


On Sept. 25, 2016, 9:36 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52147/
> ---
> 
> (Updated Sept. 25, 2016, 9:36 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for sending the `LAUNCH_NESTED_CONTAINER`
> call to the agent to launch a child container.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52147/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52154: Avoided modifying process environment.

[Benjamin Bannier]

> On Sept. 23, 2016, 1:31 a.m., Joris Van Remoortere wrote:
> > Why this solution as opposed to ensuring we initialize at the beginning of 
> > processes like we do with `process::initialize()`?

Fixing the code in this spot is slightly easier as `openssl::initialize` would
need to be called from a number of sites. Having it being called as part of
`process::initialize` would be a viable option, but would make testing slightly
harder.


- Benjamin


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


On Sept. 26, 2016, 6:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52154/
> ---
> 
> (Updated Sept. 26, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Bugs: MESOS-6216
> https://issues.apache.org/jira/browse/MESOS-6216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code modified the process environment in order to support upgrading
> on the fly from old-style libprocess SSL variables `SSL_` to
> `LIBPROCESS_SSL_`. Modifying the process environment at this point is
> not safe as other actors might concurrently read out that same
> environment.
> 
> Instead avoid changing the process environment altogether since flags
> can just as well be read from a map.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52154/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52154: Avoided modifying process environment.

[Benjamin Bannier]

> On Sept. 22, 2016, 2:15 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp, lines 317-318
> > 
> >
> > Can we have a short comment here explaining that `load` overload? Does 
> > it still even try to check the process environment or is it totally relying 
> > on that map being a complete environment view?

Thanks for bringing this up. I went and looked more carefully at how
`FlagsBase::extract` is implemented and found that it actually does some
perform some slightly specific magic where the case of environment variables
does partly ignore case. I prefixed this review with
https://reviews.apache.org/r/52256/ which exposes `FlagsBase::extract` so we
can avoid reimplementing this code here.


- Benjamin


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


On Sept. 26, 2016, 6:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52154/
> ---
> 
> (Updated Sept. 26, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Bugs: MESOS-6216
> https://issues.apache.org/jira/browse/MESOS-6216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code modified the process environment in order to support upgrading
> on the fly from old-style libprocess SSL variables `SSL_` to
> `LIBPROCESS_SSL_`. Modifying the process environment at this point is
> not safe as other actors might concurrently read out that same
> environment.
> 
> Instead avoid changing the process environment altogether since flags
> can just as well be read from a map.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52154/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52148: Added support for killing child containers to the default executor.

[Vinod Kone]

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


Fix it, then Ship it!





src/launcher/default_executor.cpp (line 380)


These should be `LOG()` statements. See my comments in the earlier review.



src/launcher/default_executor.cpp (line 389)


sorry, missed it earlier; why should shutdown be ignored if no task group 
is launched?

looks this should just call `__shutdown()` after setting `shuttingDown = 
true`?



src/launcher/default_executor.cpp (line 424)


// NOTE: We capture `connection` ...

That said I don't follow why `connection` will get disconnected if we don't 
capture it? Do you mean because it goes out of scope? AFAICT, it is used by 
`kill` calls which makes post requests.



src/launcher/default_executor.cpp (line 448)


s/Committing suicide/Terminating after/



src/launcher/default_executor.cpp (line 479)


LOG(WARNING).

Remember that this is an executor implementation and not a library. So you 
can be generous with using `LOG`.


- Vinod Kone


On Sept. 25, 2016, 10:05 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52148/
> ---
> 
> (Updated Sept. 25, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change implements support for killing child containers via
> the `KILL_NESTED_CONTAINER` call on the Agent API. This is
> triggered when the executor receives a `KILL`/`SHUTDOWN`
> event. Currently, only SIGKILL is supported. Also, the specifying
> custom kill policies is not yet supported.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52148/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52235: Supported mesos containerizer recover to be nested aware.

[Gilbert Song]

> On Sept. 25, 2016, 12:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 705
> > 
> >
> > Why the extra `!containerId.has_parent` check here? Will the first 
> > check implie the second check?

Just put a more accurate check on purpose


> On Sept. 25, 2016, 12:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 800-812
> > 
> >
> > Why having a separate look for extras? Can you just inline into the 
> > first loop? Let's keep creating of `Container` and insert into its parent 
> > close together.

I did it on purpose, since it may be possible that the extra list from the 
`launcher->recover()` container parent-child relationship. However, that is a 
hashset. We cannot guarantee when we are adding the `containerId` to its parent 
children list, the parentContainerId exists in `containers_` already.


> On Sept. 25, 2016, 12:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 743-748
> > 
> >
> > We still need to call 'destroy' for a container that does not have pid 
> > to make sure it is properly cleaned up.
> > 
> > ```
> > container->status = pid.isSome()
> >   ? reap(containerId, pid.get())
> >   : Future>(None());
> >   
> > container->status->onAny(defer(self(), &Self::reaped, containerId));
> > ```

I handled the two cases as orphans:
1. no pid containers.
2. orphans from in the `extra` list from launcher->recover().

they are cleaned up at the end of contaienrizer::_recover(). why do we call 
`reaped()` and destroy twice?


- Gilbert


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


On Sept. 24, 2016, 11:50 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52235/
> ---
> 
> (Updated Sept. 24, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported mesos containerizer recover to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 16f9e3e92e90fe7f8a0ebd24e567800e1f285bc9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 144b0db501d40d4e0bba12672723616bedd76e7e 
> 
> Diff: https://reviews.apache.org/r/52235/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51560: Deprecated using health checks without setting the type.

[Jiang Yan Xu]

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




src/health-check/health_checker.cpp (lines 185 - 189)


How about the following?

```
if (check.has_command() && !check.has_http()) {
  check.set_type(HealthCheck::COMMAND);
} else if (check.has_http() && !check.has_command()) {
  check.set_type(HealthCheck::HTTP);
} else {
...
}
```

It's obviously problematic to specify both but here we need to ensure that 
the behavior doesn't depend on the fact we look at `has_command()` first.

The other thing is that it not ideal that we need to do this in two 
different places. In the codebase we have been more consistently doing the 
following:

1. Fill in missing fields for backwards compatibility and then
2. Keep the rest of the code free from such concerns.

[One 
example](https://github.com/apache/mesos/blob/ec4c81a12559030791334359e7e1e2b6565cce01/src/master/master.cpp#L4066)

Logically this block of code could be put directly below this example, 
i.e., just before task validation.

```
TaskInfo task_(task);
if (task.has_executor() && !task.executor().has_framework_id()) {
task_.mutable_executor()
->mutable_framework_id()->CopyFrom(framework->id());
}

if (check.has_command()) {
  check.set_type(HealthCheck::COMMAND);
} else if (check.has_http()) {
  check.set_type(HealthCheck::HTTP);
}
```

Furthermore, it would be better if we extract these lines into a method.

```
TaskInfo adapt(const TaskInfo& task);


which takes care of all (past and future) such adjustments. I am not sure 
if devolve is the right place and we can put a TODO here and spend more time 
thinking about it outside this RR.



src/tests/health_check_tests.cpp (line 212)


s/compatibility reasons/backwards compatibility/ so it's more clear.



src/tests/health_check_tests.cpp (line 226)


s/compatibility reasons/backwards compatibility/ so it's more clear.


- Jiang Yan Xu


On Sept. 23, 2016, 8:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Sept. 23, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, Silas Snider, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Health checks must supply set type field from now on. Additionally,
> `HealthCheck.HTTP` message has been renamed to
> `HealthCheck.HttpCheckInfo` to avoid naming collisions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 736725c4ef954ece8580f383cfd31d289795903f 
>   src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51560: Deprecated using health checks without setting the type.

[Jiang Yan Xu]

> On Sept. 26, 2016, 10:22 a.m., Jiang Yan Xu wrote:
> > src/health-check/health_checker.cpp, lines 185-189
> > 
> >
> > How about the following?
> > 
> > ```
> > if (check.has_command() && !check.has_http()) {
> >   check.set_type(HealthCheck::COMMAND);
> > } else if (check.has_http() && !check.has_command()) {
> >   check.set_type(HealthCheck::HTTP);
> > } else {
> > ...
> > }
> > ```
> > 
> > It's obviously problematic to specify both but here we need to ensure 
> > that the behavior doesn't depend on the fact we look at `has_command()` 
> > first.
> > 
> > The other thing is that it not ideal that we need to do this in two 
> > different places. In the codebase we have been more consistently doing the 
> > following:
> > 
> > 1. Fill in missing fields for backwards compatibility and then
> > 2. Keep the rest of the code free from such concerns.
> > 
> > [One 
> > example](https://github.com/apache/mesos/blob/ec4c81a12559030791334359e7e1e2b6565cce01/src/master/master.cpp#L4066)
> > 
> > Logically this block of code could be put directly below this example, 
> > i.e., just before task validation.
> > 
> > ```
> > TaskInfo task_(task);
> > if (task.has_executor() && !task.executor().has_framework_id()) {
> > task_.mutable_executor()
> > ->mutable_framework_id()->CopyFrom(framework->id());
> > }
> > 
> > if (check.has_command()) {
> >   check.set_type(HealthCheck::COMMAND);
> > } else if (check.has_http()) {
> >   check.set_type(HealthCheck::HTTP);
> > }
> > ```
> > 
> > Furthermore, it would be better if we extract these lines into a method.
> > 
> > ```
> > TaskInfo adapt(const TaskInfo& task);
> > 
> > 
> > which takes care of all (past and future) such adjustments. I am not 
> > sure if devolve is the right place and we can put a TODO here and spend 
> > more time thinking about it outside this RR.

For the TODO I meant the "refactor into a method" part specifically.


- Jiang Yan


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


On Sept. 23, 2016, 8:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Sept. 23, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, Silas Snider, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Health checks must supply set type field from now on. Additionally,
> `HealthCheck.HTTP` message has been renamed to
> `HealthCheck.HttpCheckInfo` to avoid naming collisions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 736725c4ef954ece8580f383cfd31d289795903f 
>   src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52235: Supported mesos containerizer recover to be nested aware.

[Jie Yu]

> On Sept. 25, 2016, 7:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 743-748
> > 
> >
> > We still need to call 'destroy' for a container that does not have pid 
> > to make sure it is properly cleaned up.
> > 
> > ```
> > container->status = pid.isSome()
> >   ? reap(containerId, pid.get())
> >   : Future>(None());
> >   
> > container->status->onAny(defer(self(), &Self::reaped, containerId));
> > ```
> 
> Gilbert Song wrote:
> I handled the two cases as orphans:
> 1. no pid containers.
> 2. orphans from in the `extra` list from launcher->recover().
> 
> they are cleaned up at the end of contaienrizer::_recover(). why do we 
> call `reaped()` and destroy twice?

Ah, ic. Realized that those will be treated as orphans. Sg.


- Jie


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


On Sept. 24, 2016, 6:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52235/
> ---
> 
> (Updated Sept. 24, 2016, 6:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported mesos containerizer recover to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 16f9e3e92e90fe7f8a0ebd24e567800e1f285bc9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 144b0db501d40d4e0bba12672723616bedd76e7e 
> 
> Diff: https://reviews.apache.org/r/52235/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52154: Avoided modifying process environment.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52256, 52154]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 26, 2016, 4:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52154/
> ---
> 
> (Updated Sept. 26, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Bugs: MESOS-6216
> https://issues.apache.org/jira/browse/MESOS-6216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code modified the process environment in order to support upgrading
> on the fly from old-style libprocess SSL variables `SSL_` to
> `LIBPROCESS_SSL_`. Modifying the process environment at this point is
> not safe as other actors might concurrently read out that same
> environment.
> 
> Instead avoid changing the process environment altogether since flags
> can just as well be read from a map.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52154/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52235: Supported mesos containerizer recover to be nested aware.

[Jie Yu]

> On Sept. 25, 2016, 7:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 800-812
> > 
> >
> > Why having a separate look for extras? Can you just inline into the 
> > first loop? Let's keep creating of `Container` and insert into its parent 
> > close together.
> 
> Gilbert Song wrote:
> I did it on purpose, since it may be possible that the extra list from 
> the `launcher->recover()` container parent-child relationship. However, that 
> is a hashset. We cannot guarantee when we are adding the `containerId` to its 
> parent children list, the parentContainerId exists in `containers_` already.

hum, ic. In that sense, i'd actually prefer adding the parent child 
relationship in the end of recover.

In other words, we don't set container->children until the end. In the end, we 
just scan `containers_` and construct the parent/child relationship.

In that way, we no longer has that limitation for getContainerIds to return 
pre-order walk result.


- Jie


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


On Sept. 24, 2016, 6:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52235/
> ---
> 
> (Updated Sept. 24, 2016, 6:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported mesos containerizer recover to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 16f9e3e92e90fe7f8a0ebd24e567800e1f285bc9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 144b0db501d40d4e0bba12672723616bedd76e7e 
> 
> Diff: https://reviews.apache.org/r/52235/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52235: Supported mesos containerizer recover to be nested aware.

[Jie Yu]

- Jie


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


On Sept. 24, 2016, 6:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52235/
> ---
> 
> (Updated Sept. 24, 2016, 6:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported mesos containerizer recover to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 16f9e3e92e90fe7f8a0ebd24e567800e1f285bc9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 144b0db501d40d4e0bba12672723616bedd76e7e 
> 
> Diff: https://reviews.apache.org/r/52235/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52250: Added test case `HealthCheckTest.HealthyTaskViaHTTP`.

[haosdent huang]

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

(Updated Sept. 26, 2016, 6:36 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Fix test case error.


Repository: mesos


Description
---

Added test case `HealthCheckTest.HealthyTaskViaHTTP`.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 

Diff: https://reviews.apache.org/r/52250/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 52253: Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.

[haosdent huang]

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

(Updated Sept. 26, 2016, 6:36 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


Repository: mesos


Description
---

Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 

Diff: https://reviews.apache.org/r/52253/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 52251: Added test case `HealthCheckTest.HealthyTaskViaTCP`.

[haosdent huang]

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

(Updated Sept. 26, 2016, 6:36 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Fix test case error.


Repository: mesos


Description
---

Added test case `HealthCheckTest.HealthyTaskViaTCP`.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 

Diff: https://reviews.apache.org/r/52251/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 52252: Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaHTTP`.

[haosdent huang]

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

(Updated Sept. 26, 2016, 6:36 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Fix test case error.


Repository: mesos


Description
---

Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaHTTP`.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 

Diff: https://reviews.apache.org/r/52252/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 52149: Added support for waiting on child containers to the default executor.

[Vinod Kone]

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


Fix it, then Ship it!





src/launcher/default_executor.cpp (line 103)


log a statement here?



src/launcher/default_executor.cpp (line 462)


LOG(INFO) maybe because it is making an API call?



src/launcher/default_executor.cpp (line 827)


Add a comment here for why the delay is needed?


- Vinod Kone


On Sept. 26, 2016, 4:31 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52149/
> ---
> 
> (Updated Sept. 26, 2016, 4:31 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for waiting on child containers via the
> `WAIT_NESTED_CONTAINER` call on the Agent API. If the connection
> fails due to a temporary network blip, it reconnects with the
> agent.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52149/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52253: Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52250, 52251, 52252, 52253]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 26, 2016, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52253/
> ---
> 
> (Updated Sept. 26, 2016, 6:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp e6b02f23dc1b1b84381ab0af0e3df5918b60ae40 
> 
> Diff: https://reviews.apache.org/r/52253/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52196: Fixed warnings in `read/write` functions.

[Joseph Wu]

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




3rdparty/stout/include/stout/os/windows/read.hpp (line 32)


This should probably be a `CHECK_LE` instead of an assert.



3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 31)


What's this used for?  Is MSVC warning because of the bit shifting (`offset 
>> 32`) below?



3rdparty/stout/include/stout/os/windows/write.hpp (line 33)


`CHECK_LE` here too.


- Joseph Wu


On Sept. 22, 2016, 10:05 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52196/
> ---
> 
> (Updated Sept. 22, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in `read/write` functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> cb0abf70307f0dbba0b8f68e884df199b0359186 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 4c5178e1f3696906aa1cf93b8ca450f61ba79355 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 705ad03ee58f9cc822ec1ed25fd10f39059717d4 
> 
> Diff: https://reviews.apache.org/r/52196/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52196: Fixed warnings in `read/write` functions.

[Joseph Wu]

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




3rdparty/stout/include/stout/os/windows/read.hpp (line 35)


You wrapped the `::recv` in an earlier patch.  Want to use it here?  
(`net::recv`)


- Joseph Wu


On Sept. 22, 2016, 10:05 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52196/
> ---
> 
> (Updated Sept. 22, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in `read/write` functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> cb0abf70307f0dbba0b8f68e884df199b0359186 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 4c5178e1f3696906aa1cf93b8ca450f61ba79355 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 705ad03ee58f9cc822ec1ed25fd10f39059717d4 
> 
> Diff: https://reviews.apache.org/r/52196/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52194: Added `socket` functions to translate parameters.

[Joseph Wu]

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


Fix it, then Ship it!




I can tweak these before committing.


3rdparty/stout/include/stout/os/posix/socket.hpp (line 23)


s/os/net/



3rdparty/stout/include/stout/os/windows/socket.hpp (line 143)


I would add a quick note about these wrappers:
```
NOTE: The below wrappers are used to silence some implicit type casting 
warnings.
```



3rdparty/stout/include/stout/os/windows/socket.hpp (line 145)


You can use `CHECK_LE(addrlen, INT32_MAX);` instead.

Ditto below.


- Joseph Wu


On Sept. 22, 2016, 10:04 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52194/
> ---
> 
> (Updated Sept. 22, 2016, 10:04 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `socket` functions to translate parameters.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/socket.hpp 
> ab5f62a67c3adfe4d0c4c3532f9c2b4d43336c48 
>   3rdparty/stout/include/stout/os/windows/socket.hpp 
> d564a2bcc78570beb76a71cf35f3902333cd99ea 
> 
> Diff: https://reviews.apache.org/r/52194/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52195: Used functions from net:: namespace.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 10:05 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52195/
> ---
> 
> (Updated Sept. 22, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used functions from net:: namespace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 0dd1b6d14e66a74172d1b077353dcc3391950b08 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 1cc5e8831dbcb096c135b4cb35d1df5efcf4f622 
> 
> Diff: https://reviews.apache.org/r/52195/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52004: Supported docker runtime isolator to be nested aware.

[Gilbert Song]

> On Sept. 18, 2016, 2:03 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 77-82
> > 
> >
> > We can remove this. The default behavior is this.
> 
> Guangya Liu wrote:
> There is already a patch here 
> https://reviews.apache.org/r/49234/diff/2#index_header

Hey Guangya, thanks for the patch. I want to include your cleanup in my patch, 
which might make it easier for the shepherd.


- Gilbert


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


On Sept. 18, 2016, 10:30 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52004/
> ---
> 
> (Updated Sept. 18, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6190
> https://issues.apache.org/jira/browse/MESOS-6190
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported docker runtime isolator to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> ac0397f65bb2a675ad1eae0f7cfa95f10145fc63 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> b589cd691ae6aacd2dcd00878e43d58f15abfe11 
> 
> Diff: https://reviews.apache.org/r/52004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51999: Refactor parsing of resources.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 8:58 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Refactored `Resources::parse()` into 2 separate static functions:
1. Resources::fromJSONString() to parse JSON representation of
   resources.
2. Resources::fromSimpleString() to parse text representation of
   resources.

Since these 2 new functions return a `Try>`, the
existing `Resources::parse()` implicitly converts that to a
`Resources` object. This refactor is done to retrieve all resources
(include empty resources) required for auto detection of root
and MOUNT disks.


Diffs (updated)
-

  include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 
  include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 
  src/common/resources.cpp 0774ff0669e831494d5b12b88e19dfa0a4a3f757 
  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 
  src/v1/resources.cpp 62a644ebbd13cfc0862bd118ba16c43e0f6aaf90 

Diff: https://reviews.apache.org/r/51999/diff/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52002: Added helper methods to determine types of disk resources.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 8:58 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added helper methods to determine types of disk resources.


Diffs (updated)
-

  include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 
  include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 
  src/common/resources.cpp 0774ff0669e831494d5b12b88e19dfa0a4a3f757 
  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 
  src/v1/resources.cpp 62a644ebbd13cfc0862bd118ba16c43e0f6aaf90 

Diff: https://reviews.apache.org/r/52002/diff/


Testing (updated)
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52197: Used intermediate functions.

[Joseph Wu]

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



This one should be squashed into: https://reviews.apache.org/r/52195/
I can do this before committing.

- Joseph Wu


On Sept. 22, 2016, 9:56 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52197/
> ---
> 
> (Updated Sept. 22, 2016, 9:56 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used `socket` intermediate functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/io.cpp d9c60754b1eaf83b1f5626a220f431cadeca541a 
> 
> Diff: https://reviews.apache.org/r/52197/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 51880: Added unit tests to determine disk size for MOUNT disks.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 8:59 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added unit tests to determine disk size for MOUNT disks.


Diffs (updated)
-

  src/Makefile.am 410b5f2e5bc50a5aa7856645c8cf4a3e43505a84 
  src/tests/containerizer/common_containerizer_tests.cpp PRE-CREATION 
  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 

Diff: https://reviews.apache.org/r/51880/diff/


Testing
---

All tests including the additional tests in this RR passed.


Thanks,

Anindya Sinha

Re: Review Request 51879: Autodetect value of resource when not specified in static resources.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 8:59 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Fixed the build issue on linux platform in parsing for gpus.


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


Repository: mesos


Description
---

When static resources indicate resources with a positive size, we use
that for the resources on the agent. However, --resources can include
resources with no size, which indicates that mesos agent determine the
size of those resources from the agent and uses that information. For
disk resources, this is not allowed for PATH disks since PATH disks
can be carved into smaller chunks and we cannot assume that the whole
physical disk is available to the PATH disk.

With this change, JSON or textual representation for disk resources
that specify scalar value of 0 would not result in an error, but those
resources will not be accounted for until a valid size is determined
for such resources. A scalar value of -1 in JSON or textual formats
still results in an invalid resource.


Diffs (updated)
-

  include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 
  include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 
  src/common/resources.cpp 0774ff0669e831494d5b12b88e19dfa0a4a3f757 
  src/slave/containerizer/containerizer.cpp 
d46882baa904fd439bffb23c324828b777228f1c 
  src/slave/containerizer/mesos/isolators/gpu/allocator.hpp 
b2eabfebef99ccebef427d144bb816adc0175ecf 
  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 
  src/v1/resources.cpp 62a644ebbd13cfc0862bd118ba16c43e0f6aaf90 

Diff: https://reviews.apache.org/r/51879/diff/


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52071: Updated docs to handle resources with no size in agent flags.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 9 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Fixed the commit summary, and the fix in a RR in this chain should fix the 
build issue.


Summary (updated)
-

Updated docs to handle resources with no size in agent flags.


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


Repository: mesos


Description (updated)
---

When a resource with no size is specified in `--resources` startup
flag of mesos agent, the size is auto detected by the agent. This
is enabled for all known resource types (except gpus).
For disk resources, this is done for both root disks as well as
MOUNT disks, but is not allowed for PATH disks since PATH disks can
be carved into smaller volumes.


Diffs (updated)
-

  docs/attributes-resources.md 2caa44927d45c0ab13f8160794b50f4fde3711aa 

Diff: https://reviews.apache.org/r/52071/diff/


Testing
---

Documentation change only. All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52196: Fixed warnings in `read/write` functions.

[Daniel Pravat]

> On Sept. 26, 2016, 8:38 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/windows/sendfile.hpp, line 31
> > 
> >
> > What's this used for?  Is MSVC warning because of the bit shifting 
> > (`offset >> 32`) below?

Yes.


- Daniel


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


On Sept. 23, 2016, 5:05 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52196/
> ---
> 
> (Updated Sept. 23, 2016, 5:05 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in `read/write` functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> cb0abf70307f0dbba0b8f68e884df199b0359186 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 4c5178e1f3696906aa1cf93b8ca450f61ba79355 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 705ad03ee58f9cc822ec1ed25fd10f39059717d4 
> 
> Diff: https://reviews.apache.org/r/52196/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52196: Fixed warnings in `read/write` functions.

[Daniel Pravat]

> On Sept. 26, 2016, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/windows/read.hpp, line 35
> > 
> >
> > You wrapped the `::recv` in an earlier patch.  Want to use it here?  
> > (`net::recv`)

Good catch (bad merge?). recv() takes the lengths as `int` while _read() takes 
it as `unsigned int`.


- Daniel


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


On Sept. 23, 2016, 5:05 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52196/
> ---
> 
> (Updated Sept. 23, 2016, 5:05 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in `read/write` functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> cb0abf70307f0dbba0b8f68e884df199b0359186 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 4c5178e1f3696906aa1cf93b8ca450f61ba79355 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 705ad03ee58f9cc822ec1ed25fd10f39059717d4 
> 
> Diff: https://reviews.apache.org/r/52196/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52196: Fixed warnings in `read/write` functions.

[Daniel Pravat]

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

(Updated Sept. 26, 2016, 9:15 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in `read/write` functions.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/read.hpp 
cb0abf70307f0dbba0b8f68e884df199b0359186 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
4c5178e1f3696906aa1cf93b8ca450f61ba79355 
  3rdparty/stout/include/stout/os/windows/write.hpp 
705ad03ee58f9cc822ec1ed25fd10f39059717d4 

Diff: https://reviews.apache.org/r/52196/diff/


Testing
---

OSX: make check
Windows: build


Thanks,

Daniel Pravat

Re: Review Request 52194: Added functions to translate parameters.

[Daniel Pravat]

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

(Updated Sept. 26, 2016, 9:21 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Summary (updated)
-

Added functions to translate parameters.


Repository: mesos


Description (updated)
---

Added functions to translate parameters.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/socket.hpp 
ab5f62a67c3adfe4d0c4c3532f9c2b4d43336c48 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
d564a2bcc78570beb76a71cf35f3902333cd99ea 

Diff: https://reviews.apache.org/r/52194/diff/


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Re: Review Request 52195: Used functions from `net::` namespace.

[Daniel Pravat]

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

(Updated Sept. 26, 2016, 9:37 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Summary (updated)
-

Used functions from `net::` namespace.


Repository: mesos


Description (updated)
---

Used functions from `net::` namespace.


Diffs (updated)
-

  3rdparty/libprocess/include/process/network.hpp 
0dd1b6d14e66a74172d1b077353dcc3391950b08 
  3rdparty/libprocess/src/poll_socket.cpp 
1cc5e8831dbcb096c135b4cb35d1df5efcf4f622 

Diff: https://reviews.apache.org/r/52195/diff/


Testing
---

OSX: make check
Windows: build


Thanks,

Daniel Pravat

Re: Review Request 52062: Fixed warnings in `numify.hpp`.

[Daniel Pravat]

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

(Updated Sept. 26, 2016, 9:37 p.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

When the template is instatiated for unsigned scalars
the unary negation generates warnings.


Diffs (updated)
-

  3rdparty/stout/include/stout/numify.hpp 
c174fcb8cb9d809f443e44058f07b58751bed9dd 

Diff: https://reviews.apache.org/r/52062/diff/


Testing
---

Windows: build


Thanks,

Daniel Pravat

Review Request 52284: Implement more quota validation tests.

[Zhitao Li]

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

Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


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


Repository: mesos


Description
---

Implement more quota validation tests.


Diffs
-

  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 

Diff: https://reviews.apache.org/r/52284/diff/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52165: Disabled default executor tests for now.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 25, 2016, 10:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52165/
> ---
> 
> (Updated Sept. 25, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just a temporary set up to make the ReviewBot happy.
> Eventually, when the launcher changes land and we are able to
> launch child containers these tests would pass!
> 
> Please note that it is currently a bit hard to unit test
> the default executor as we don't have the equivalent of
> a `MockMesosContainerizer` that actually launches an
> executor. `TestContainerizer` never launches an executor though
> we can refactor it to suport v1 executors.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 786fbe3baac3c1542b318a26363213a4a1945ddd 
> 
> Diff: https://reviews.apache.org/r/52165/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52200: Fixed warnings in system.hpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:57 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52200/
> ---
> 
> (Updated Sept. 22, 2016, 9:57 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in system.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/system.hpp 
> 2a61b8e7fcd797a22ada6322932baefc485b7d56 
> 
> Diff: https://reviews.apache.org/r/52200/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52201: Fixed warnings in timer.hpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:55 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52201/
> ---
> 
> (Updated Sept. 22, 2016, 9:55 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in timer.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/timer.hpp 
> d60b9aec78210e30cce4c053939f6fecf36ba938 
> 
> Diff: https://reviews.apache.org/r/52201/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52147: Added support for launching child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 26, 2016, 10:25 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review Comments, NNFR.


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


Repository: mesos


Description
---

This change adds support for sending the `LAUNCH_NESTED_CONTAINER`
call to the agent to launch a child container.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

Diff: https://reviews.apache.org/r/52147/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

reviews@mesos.apache.org

[Joseph Wu]

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


Fix it, then Ship it!




I can tweak this before committing.


3rdparty/libprocess/src/decoder.hpp (line 242)


`CHECK_LE`



3rdparty/libprocess/src/decoder.hpp (line 434)


`CHECK_LE`.


- Joseph Wu


On Sept. 22, 2016, 9:55 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52202/
> ---
> 
> (Updated Sept. 22, 2016, 9:55 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in encoder&decoder.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> ccf12ac8ae05141b4324ef2b4599a6bd7461c4ec 
>   3rdparty/libprocess/src/encoder.hpp 
> af083d196aa67d08f1b3299b0a15fbc7d1fd7cd5 
> 
> Diff: https://reviews.apache.org/r/52202/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52203: Fixed warnings in metrics.hpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:54 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52203/
> ---
> 
> (Updated Sept. 22, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in metrics.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
> 
> Diff: https://reviews.apache.org/r/52203/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52204: Fixed warnings in socket.hpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:53 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52204/
> ---
> 
> (Updated Sept. 22, 2016, 9:53 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in socket.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/socket.cpp 1e495181f3e43d4a2d6cdf8aa8c9605dfd52c81b 
> 
> Diff: https://reviews.apache.org/r/52204/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52004: Supported docker runtime isolator to be nested aware.

[Guangya Liu]

> On 九月 18, 2016, 9:03 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 77-82
> > 
> >
> > We can remove this. The default behavior is this.
> 
> Guangya Liu wrote:
> There is already a patch here 
> https://reviews.apache.org/r/49234/diff/2#index_header
> 
> Gilbert Song wrote:
> Hey Guangya, thanks for the patch. I want to include your cleanup in my 
> patch, which might make it easier for the shepherd.

OK, I have disacard my patch here 
https://reviews.apache.org/r/49234/diff/2#index_header


- Guangya


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


On 九月 18, 2016, 5:30 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52004/
> ---
> 
> (Updated 九月 18, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6190
> https://issues.apache.org/jira/browse/MESOS-6190
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported docker runtime isolator to be nested aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> ac0397f65bb2a675ad1eae0f7cfa95f10145fc63 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> b589cd691ae6aacd2dcd00878e43d58f15abfe11 
> 
> Diff: https://reviews.apache.org/r/52004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52147: Added support for launching child containers to the default executor.

[Anand Mazumdar]

> On Sept. 26, 2016, 3:56 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 241
> > 
> >
> > I would change these to LOG(ERROR) because dropping a task group launch 
> > is an important enough event that we want to surface without expecting 
> > users to set GLOG_v.
> > 
> > here and everywhere else.

As per our offline discussion, I changed this to a `LOG(WARNING)` to be 
consistent with similar occurences on the master/agent.


- Anand


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


On Sept. 26, 2016, 10:25 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52147/
> ---
> 
> (Updated Sept. 26, 2016, 10:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for sending the `LAUNCH_NESTED_CONTAINER`
> call to the agent to launch a child container.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52147/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52148: Added support for killing child containers to the default executor.

[Anand Mazumdar]

> On Sept. 26, 2016, 4:45 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 389
> > 
> >
> > sorry, missed it earlier; why should shutdown be ignored if no task 
> > group is launched?
> > 
> > looks this should just call `__shutdown()` after setting `shuttingDown 
> > = true`?

Good catch. I had blatantly copied this over from the command executor. Looks 
like we need to fix it there too!


> On Sept. 26, 2016, 4:45 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 424
> > 
> >
> > // NOTE: We capture `connection` ...
> > 
> > That said I don't follow why `connection` will get disconnected if we 
> > don't capture it? Do you mean because it goes out of scope? AFAICT, it is 
> > used by `kill` calls which makes post requests.

`Connection` follows the RAII idiom and disconnects itself when its last ref 
counted instance is gone. We need to keep a reference to it to avoid 
disconnection and not able to send the inflight pipelined `POST` requests.


> On Sept. 26, 2016, 4:45 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 497
> > 
> >
> > LOG(WARNING).
> > 
> > Remember that this is an executor implementation and not a library. So 
> > you can be generous with using `LOG`.

I don't think using `VLOG()` has anything to do with it being an implementation 
or a library. However, I understand that this is what we have been following in 
Mesos in practice and would be good to be consistent. I removed all the 
occurences minus the `connectionId` mismatch logging from `VLOG()`.


- Anand


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


On Sept. 25, 2016, 10:05 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52148/
> ---
> 
> (Updated Sept. 25, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change implements support for killing child containers via
> the `KILL_NESTED_CONTAINER` call on the Agent API. This is
> triggered when the executor receives a `KILL`/`SHUTDOWN`
> event. Currently, only SIGKILL is supported. Also, the specifying
> custom kill policies is not yet supported.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52148/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52148: Added support for killing child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 26, 2016, 10:33 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments, NNFR.


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


Repository: mesos


Description
---

This change implements support for killing child containers via
the `KILL_NESTED_CONTAINER` call on the Agent API. This is
triggered when the executor receives a `KILL`/`SHUTDOWN`
event. Currently, only SIGKILL is supported. Also, the specifying
custom kill policies is not yet supported.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

Diff: https://reviews.apache.org/r/52148/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52206: Fixed warnings in os.hpp.

[Joseph Wu]

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


Fix it, then Ship it!




I can tweak these before committing.


3rdparty/stout/include/stout/windows/os.hpp (line 154)


`CHECK_LE`.



3rdparty/stout/include/stout/windows/os.hpp (lines 156 - 158)


`processes.data()` should go on the next line, with 4 spaces of indent.


- Joseph Wu


On Sept. 22, 2016, 9:52 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52206/
> ---
> 
> (Updated Sept. 22, 2016, 9:52 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in os.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> fb341348d842513f7479a9f00880c8b63dd49fb6 
> 
> Diff: https://reviews.apache.org/r/52206/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52149: Added support for waiting on child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 26, 2016, 10:42 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments, NNFR.


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


Repository: mesos


Description
---

This change adds support for waiting on child containers via the
`WAIT_NESTED_CONTAINER` call on the Agent API. If the connection
fails due to a temporary network blip, it reconnects with the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

Diff: https://reviews.apache.org/r/52149/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52165: Fixed default executor tests.

[Anand Mazumdar]

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

(Updated Sept. 26, 2016, 10:51 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated summary and description.


Summary (updated)
-

Fixed default executor tests.


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


Repository: mesos


Description (updated)
---

This change modifies a couple of default executor tests that
would fail otherwise after the launch/wait/destroy child
container changes. The changes are as under:
- Disable AuthN on the agent allowing the executor to connect.
- Fix an incorrect assumption in a test that status updates for
  different tasks would be sent in order.

Disabled the tests for now to make ReviewBot happy.
Eventually, when the launcher changes land and we are able to
launch child containers these tests would pass!


Diffs
-

  src/tests/default_executor_tests.cpp 786fbe3baac3c1542b318a26363213a4a1945ddd 

Diff: https://reviews.apache.org/r/52165/diff/


Testing (updated)
---

make check (on Gilbert's branch that has the launcher changes)


Thanks,

Anand Mazumdar

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Jiang Yan Xu]

> On Sept. 19, 2016, 1:35 p.m., Joseph Wu wrote:
> > src/slave/containerizer/fetcher.cpp, lines 773-775
> > 
> >
> > (You'll want to update this comment.)
> > 
> > There isn't always a stdout/stderr file in the sandbox, as this depends 
> > on the ContainerLogger module.  Would non-recursively chown-ing the sandbox 
> > directory work as well?  (This is already done when the agent first creates 
> > the sandbox.)
> 
> Megha Sharma wrote:
> The FetcherProcess::run(...) already creates the stdout and stderr and 
> then recursively changes the ownership of the sandbox to make sure these 2 
> files have the right ownership which I modified to change the ownership of 
> just these 2 files exclusively and not the entire sandbox. So, at this point 
> the stdout and stderr are already created. 
> Also, like you mentioned the agent is already doing a chown while 
> creating the sandbox directory so the sandbox is already owned by the desired 
> user. The concern that I am addressing here is avoiding unnecessary chowning 
> of sandbox so files laid out by other entities don't change ownership but we 
> still anyway need to set the right owners for stdout and stderr here.
> 
> Joseph Wu wrote:
> Right, forgot about the fetcher's own stdout/stderr :)
> 
> I'd argue it makes sense to not do any chown-ing at all.  The 
> stdout/stderr files are technically created by other entities than the 
> executor (i.e. the fetcher, agent, and logger module).  Not sure if this 
> breaks any expectations in the fetcher though...
> 
> Megha Sharma wrote:
> Since now the fetcher is being run as task user as a result of jira 
> https://issues.apache.org/jira/browse/MESOS-5845 so these 2 files need to be 
> chowned to the same user for the fetcher to be able to write to them.

@Joseph: Given the way it works today, is the fetcher stdout/stderr handled by 
the container logger at all? It doesn't look so but should it?

The chown (recursive or not) does look out of place in the fetcher. Since we 
always have a container logger now (default to SandboxContainerLogger which has 
no runtime component but only exists during container setup) it seems 
appropriate for the logger to assume the responsibility to set up stderr and 
stdout FDs (backed by files or not) and make sure the contained processes 
(fetcher and executor both now run under the task user) **can write** to them, 
right?


- Jiang Yan


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


On Sept. 20, 2016, 4:32 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52058/
> ---
> 
> (Updated Sept. 20, 2016, 4:32 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5218
> https://issues.apache.org/jira/browse/MESOS-5218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher code changes the ownership of entire sandbox directory
> recursively to the taskuser and as a resut also changes the
> ownership of files laid out by other entities in the sandbox.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 4d3fc521bf8a7c438c48e31c549f108ac3602b3f 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 96e24500a12825161553eb050da389088b122695 
> 
> Diff: https://reviews.apache.org/r/52058/diff/
> 
> 
> Testing
> ---
> 
> make check on linux
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 52208: Fixed warnings in health_checker.cpp.

[Joseph Wu]

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


Fix it, then Ship it!




I can tweak this before committing.


src/health-check/health_checker.cpp (lines 185 - 187)


The indented lines here should be aligned with the end of `delay(`.



src/health-check/health_checker.cpp (lines 592 - 594)


Ditto here.


- Joseph Wu


On Sept. 22, 2016, 9:50 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52208/
> ---
> 
> (Updated Sept. 22, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in health_checker.cpp.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 758cbb382b08743f518022ba66eaafbea7615592 
> 
> Diff: https://reviews.apache.org/r/52208/diff/
> 
> 
> Testing
> ---
> 
> Windows: buils
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52207: Fixed warnings in fs.hpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:50 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52207/
> ---
> 
> (Updated Sept. 22, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in fs.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/fs.hpp 
> 99887b1047aedf1d814690cbf826469cb0e4fefa 
> 
> Diff: https://reviews.apache.org/r/52207/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Review Request 52285: Added 'volume/sandbox_path' isolator.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Joseph Wu.


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


Repository: mesos


Description
---

Added 'volume/sandbox_path' isolator.


Diffs
-

  src/CMakeLists.txt 981b36e55785b9de00a79693d5b25fe66fccb710 
  src/Makefile.am 410b5f2e5bc50a5aa7856645c8cf4a3e43505a84 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/52285/diff/


Testing
---

To be added. Wait for the nested container support.


Thanks,

Jie Yu

Review Request 52286: Enabled 'volume/sandbox_path' isolator in MesosContainerizer.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Joseph Wu.


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


Repository: mesos


Description
---

Enabled 'volume/sandbox_path' isolator in MesosContainerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
a88e4039da7319bc5b6c4003b67ef7c23ad5c320 

Diff: https://reviews.apache.org/r/52286/diff/


Testing
---

To be added. Wait for the nested container support.


Thanks,

Jie Yu

Re: Review Request 51277: Added an 'ns::clone' helper.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 25, 2016, 4:33 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51277/
> ---
> 
> (Updated Sept. 25, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an 'ns::clone' helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 2d6c359ed24d6e964882f34df60d8182491a27c9 
>   src/tests/containerizer/ns_tests.cpp 
> 6aa5ade9a25356057ace609fb875f070aca1ec0e 
> 
> Diff: https://reviews.apache.org/r/51277/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 51278: Refactored LinuxLauncher to be nested container aware.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 25, 2016, 4:08 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51278/
> ---
> 
> (Updated Sept. 25, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored LinuxLauncher to be nested container aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 2cd1e4d4b6d9e656c2447a9d3ab8e1f49ba5fffa 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> f27757032cc28be0f0067bed045536431dde4d6c 
> 
> Diff: https://reviews.apache.org/r/51278/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Joseph Wu]

> On Sept. 19, 2016, 1:35 p.m., Joseph Wu wrote:
> > src/slave/containerizer/fetcher.cpp, lines 773-775
> > 
> >
> > (You'll want to update this comment.)
> > 
> > There isn't always a stdout/stderr file in the sandbox, as this depends 
> > on the ContainerLogger module.  Would non-recursively chown-ing the sandbox 
> > directory work as well?  (This is already done when the agent first creates 
> > the sandbox.)
> 
> Megha Sharma wrote:
> The FetcherProcess::run(...) already creates the stdout and stderr and 
> then recursively changes the ownership of the sandbox to make sure these 2 
> files have the right ownership which I modified to change the ownership of 
> just these 2 files exclusively and not the entire sandbox. So, at this point 
> the stdout and stderr are already created. 
> Also, like you mentioned the agent is already doing a chown while 
> creating the sandbox directory so the sandbox is already owned by the desired 
> user. The concern that I am addressing here is avoiding unnecessary chowning 
> of sandbox so files laid out by other entities don't change ownership but we 
> still anyway need to set the right owners for stdout and stderr here.
> 
> Joseph Wu wrote:
> Right, forgot about the fetcher's own stdout/stderr :)
> 
> I'd argue it makes sense to not do any chown-ing at all.  The 
> stdout/stderr files are technically created by other entities than the 
> executor (i.e. the fetcher, agent, and logger module).  Not sure if this 
> breaks any expectations in the fetcher though...
> 
> Megha Sharma wrote:
> Since now the fetcher is being run as task user as a result of jira 
> https://issues.apache.org/jira/browse/MESOS-5845 so these 2 files need to be 
> chowned to the same user for the fetcher to be able to write to them.
> 
> Jiang Yan Xu wrote:
> @Joseph: Given the way it works today, is the fetcher stdout/stderr 
> handled by the container logger at all? It doesn't look so but should it?
> 
> The chown (recursive or not) does look out of place in the fetcher. Since 
> we always have a container logger now (default to SandboxContainerLogger 
> which has no runtime component but only exists during container setup) it 
> seems appropriate for the logger to assume the responsibility to set up 
> stderr and stdout FDs (backed by files or not) and make sure the contained 
> processes (fetcher and executor both now run under the task user) **can 
> write** to them, right?

It does make sense to pass responsibility for the fetcher's stdout/stderr into 
the ContainerLogger too.  This will give a unified logging interface, in case 
anyone wants to pipe their logs somewhere else.

In order to do this, looks like there are a couple changes needed to the 
fetcher interface and how we pass along file descriptors from the 
ContainerLogger.

(1) The fetcher will need to take some additional arguments of the form:
```
const ContainerLogger::SubprocessInfo& subprocessInfo
```
Here:
https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/fetcher.hpp#L95-L106

---

(2) The containerizers need to pass this `SubprocessInfo` from the 
ContainerLogger to Fetcher.  We can accomplish this fairly trivially in the 
Mesos Containerizer:

The logger is called here:
https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1188-L1190

And the fetcher is called inside the logger's continuation:
https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1358-L1361

Unfortunately, the Docker Containerizer has it backwards:
https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L-L1123
https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L1143-L1155

---

(3) We will need to tweak the way `SubprocessInfo::FD` is passed around.  
Currently, as soon as the FD is sent into `subprocess`, the ownership of the FD 
is transfered into the subprocess.  If we plan to send the same FD to the 
Executor and Fetcher, the fetcher cannot be allowed to take ownership.

https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/include/mesos/slave/container_logger.hpp#L79-L86


- Joseph


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


On Sept. 20, 2016, 4:32 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52058/
> ---
> 

Re: Review Request 52209: Fixed warnings in slave.cpp.

[Joseph Wu]

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


Ship it!




- Joseph Wu


On Sept. 22, 2016, 9:49 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52209/
> ---
> 
> (Updated Sept. 22, 2016, 9:49 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in slave.cpp.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 3e7832c18acf876e8139fd0758204d8af1d91a04 
> 
> Diff: https://reviews.apache.org/r/52209/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 52165: Fixed default executor tests.

[Anand Mazumdar]

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

(Updated Sept. 26, 2016, 11:28 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Make review bot run again.


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


Repository: mesos


Description
---

This change modifies a couple of default executor tests that
would fail otherwise after the launch/wait/destroy child
container changes. The changes are as under:
- Disable AuthN on the agent allowing the executor to connect.
- Fix an incorrect assumption in a test that status updates for
  different tasks would be sent in order.

Disabled the tests for now to make ReviewBot happy.
Eventually, when the launcher changes land and we are able to
launch child containers these tests would pass!


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 786fbe3baac3c1542b318a26363213a4a1945ddd 

Diff: https://reviews.apache.org/r/52165/diff/


Testing
---

make check (on Gilbert's branch that has the launcher changes)


Thanks,

Anand Mazumdar

Review Request 52287: Added a test for default executor reconnect upon an process restart.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds a test for validating that the default executor
can reconnect with the agent upon an agent process restart if
framework checkpointing is enabled.

This test is disabled for now to make ReviewBot happy but
would be enabled when the launcher changes land.


Diffs
-

  src/tests/slave_recovery_tests.cpp 618019c1e2ca87073b4b42d9dd4b228a473ef8d3 

Diff: https://reviews.apache.org/r/52287/diff/


Testing
---

make check (verified the test passes on Gilbert's branch containing the 
launcher changes)


Thanks,

Anand Mazumdar

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Jiang Yan Xu]

> On Sept. 19, 2016, 1:35 p.m., Joseph Wu wrote:
> > src/slave/containerizer/fetcher.cpp, lines 773-775
> > 
> >
> > (You'll want to update this comment.)
> > 
> > There isn't always a stdout/stderr file in the sandbox, as this depends 
> > on the ContainerLogger module.  Would non-recursively chown-ing the sandbox 
> > directory work as well?  (This is already done when the agent first creates 
> > the sandbox.)
> 
> Megha Sharma wrote:
> The FetcherProcess::run(...) already creates the stdout and stderr and 
> then recursively changes the ownership of the sandbox to make sure these 2 
> files have the right ownership which I modified to change the ownership of 
> just these 2 files exclusively and not the entire sandbox. So, at this point 
> the stdout and stderr are already created. 
> Also, like you mentioned the agent is already doing a chown while 
> creating the sandbox directory so the sandbox is already owned by the desired 
> user. The concern that I am addressing here is avoiding unnecessary chowning 
> of sandbox so files laid out by other entities don't change ownership but we 
> still anyway need to set the right owners for stdout and stderr here.
> 
> Joseph Wu wrote:
> Right, forgot about the fetcher's own stdout/stderr :)
> 
> I'd argue it makes sense to not do any chown-ing at all.  The 
> stdout/stderr files are technically created by other entities than the 
> executor (i.e. the fetcher, agent, and logger module).  Not sure if this 
> breaks any expectations in the fetcher though...
> 
> Megha Sharma wrote:
> Since now the fetcher is being run as task user as a result of jira 
> https://issues.apache.org/jira/browse/MESOS-5845 so these 2 files need to be 
> chowned to the same user for the fetcher to be able to write to them.
> 
> Jiang Yan Xu wrote:
> @Joseph: Given the way it works today, is the fetcher stdout/stderr 
> handled by the container logger at all? It doesn't look so but should it?
> 
> The chown (recursive or not) does look out of place in the fetcher. Since 
> we always have a container logger now (default to SandboxContainerLogger 
> which has no runtime component but only exists during container setup) it 
> seems appropriate for the logger to assume the responsibility to set up 
> stderr and stdout FDs (backed by files or not) and make sure the contained 
> processes (fetcher and executor both now run under the task user) **can 
> write** to them, right?
> 
> Joseph Wu wrote:
> It does make sense to pass responsibility for the fetcher's stdout/stderr 
> into the ContainerLogger too.  This will give a unified logging interface, in 
> case anyone wants to pipe their logs somewhere else.
> 
> In order to do this, looks like there are a couple changes needed to the 
> fetcher interface and how we pass along file descriptors from the 
> ContainerLogger.
> 
> (1) The fetcher will need to take some additional arguments of the form:
> ```
> const ContainerLogger::SubprocessInfo& subprocessInfo
> ```
> Here:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/fetcher.hpp#L95-L106
> 
> ---
> 
> (2) The containerizers need to pass this `SubprocessInfo` from the 
> ContainerLogger to Fetcher.  We can accomplish this fairly trivially in the 
> Mesos Containerizer:
> 
> The logger is called here:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1188-L1190
> 
> And the fetcher is called inside the logger's continuation:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1358-L1361
> 
> Unfortunately, the Docker Containerizer has it backwards:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L-L1123
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L1143-L1155
> 
> ---
> 
> (3) We will need to tweak the way `SubprocessInfo::FD` is passed around.  
> Currently, as soon as the FD is sent into `subprocess`, the ownership of the 
> FD is transfered into the subprocess.  If we plan to send the same FD to the 
> Executor and Fetcher, the fetcher cannot be allowed to take ownership.
> 
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/include/mesos/slave/container_logger.hpp#L79-L86

Thanks for the reply. In addition to passing SubprocessInfo to the fetcher, we 
have to set the perms for the files. Since it's only relavent to the sandbox 
logger, I guess we have to handle it in the sandbox logger itself? This would 
probably require the 'user' (which can't be deriv

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Anindya Sinha]

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




src/slave/containerizer/fetcher.cpp (line 765)


Add a line here.



src/slave/containerizer/fetcher.cpp (line 770)


I think this should be more like:
```
if (chownOut.isError() || chownErr.isError()) {
  os::close(out.get());
  os::close(err.get());
  return Failure(...);
}
```

And the `Failure` message can indicate which one failed, ie. `stdout` or 
`stderr` or both (if you want to expose that bit of information).



src/slave/containerizer/fetcher.cpp (line 772)


If there is an error in either `chownOut` or `chownErr`, there is a 
`os::close(out.get())` and then we return `Failure`... which means we never get 
to do a `os::close(err.get())`. So, a potential leak in file descriptor!



src/slave/containerizer/fetcher.cpp (line 774)


Also, the `Failure` message indicates the failure is with `stdout` which 
may not be true if only `chownErr.isError()` is true.



src/slave/containerizer/fetcher.cpp (line 779)


Kill this block if you consolidate the error handling for `chownOut` and 
`chownErr`.


- Anindya Sinha


On Sept. 20, 2016, 11:32 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52058/
> ---
> 
> (Updated Sept. 20, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5218
> https://issues.apache.org/jira/browse/MESOS-5218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher code changes the ownership of entire sandbox directory
> recursively to the taskuser and as a resut also changes the
> ownership of files laid out by other entities in the sandbox.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 4d3fc521bf8a7c438c48e31c549f108ac3602b3f 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 96e24500a12825161553eb050da389088b122695 
> 
> Diff: https://reviews.apache.org/r/52058/diff/
> 
> 
> Testing
> ---
> 
> make check on linux
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Joseph Wu]

> On Sept. 19, 2016, 1:35 p.m., Joseph Wu wrote:
> > src/slave/containerizer/fetcher.cpp, lines 773-775
> > 
> >
> > (You'll want to update this comment.)
> > 
> > There isn't always a stdout/stderr file in the sandbox, as this depends 
> > on the ContainerLogger module.  Would non-recursively chown-ing the sandbox 
> > directory work as well?  (This is already done when the agent first creates 
> > the sandbox.)
> 
> Megha Sharma wrote:
> The FetcherProcess::run(...) already creates the stdout and stderr and 
> then recursively changes the ownership of the sandbox to make sure these 2 
> files have the right ownership which I modified to change the ownership of 
> just these 2 files exclusively and not the entire sandbox. So, at this point 
> the stdout and stderr are already created. 
> Also, like you mentioned the agent is already doing a chown while 
> creating the sandbox directory so the sandbox is already owned by the desired 
> user. The concern that I am addressing here is avoiding unnecessary chowning 
> of sandbox so files laid out by other entities don't change ownership but we 
> still anyway need to set the right owners for stdout and stderr here.
> 
> Joseph Wu wrote:
> Right, forgot about the fetcher's own stdout/stderr :)
> 
> I'd argue it makes sense to not do any chown-ing at all.  The 
> stdout/stderr files are technically created by other entities than the 
> executor (i.e. the fetcher, agent, and logger module).  Not sure if this 
> breaks any expectations in the fetcher though...
> 
> Megha Sharma wrote:
> Since now the fetcher is being run as task user as a result of jira 
> https://issues.apache.org/jira/browse/MESOS-5845 so these 2 files need to be 
> chowned to the same user for the fetcher to be able to write to them.
> 
> Jiang Yan Xu wrote:
> @Joseph: Given the way it works today, is the fetcher stdout/stderr 
> handled by the container logger at all? It doesn't look so but should it?
> 
> The chown (recursive or not) does look out of place in the fetcher. Since 
> we always have a container logger now (default to SandboxContainerLogger 
> which has no runtime component but only exists during container setup) it 
> seems appropriate for the logger to assume the responsibility to set up 
> stderr and stdout FDs (backed by files or not) and make sure the contained 
> processes (fetcher and executor both now run under the task user) **can 
> write** to them, right?
> 
> Joseph Wu wrote:
> It does make sense to pass responsibility for the fetcher's stdout/stderr 
> into the ContainerLogger too.  This will give a unified logging interface, in 
> case anyone wants to pipe their logs somewhere else.
> 
> In order to do this, looks like there are a couple changes needed to the 
> fetcher interface and how we pass along file descriptors from the 
> ContainerLogger.
> 
> (1) The fetcher will need to take some additional arguments of the form:
> ```
> const ContainerLogger::SubprocessInfo& subprocessInfo
> ```
> Here:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/fetcher.hpp#L95-L106
> 
> ---
> 
> (2) The containerizers need to pass this `SubprocessInfo` from the 
> ContainerLogger to Fetcher.  We can accomplish this fairly trivially in the 
> Mesos Containerizer:
> 
> The logger is called here:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1188-L1190
> 
> And the fetcher is called inside the logger's continuation:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/mesos/containerizer.cpp#L1358-L1361
> 
> Unfortunately, the Docker Containerizer has it backwards:
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L-L1123
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/src/slave/containerizer/docker.cpp#L1143-L1155
> 
> ---
> 
> (3) We will need to tweak the way `SubprocessInfo::FD` is passed around.  
> Currently, as soon as the FD is sent into `subprocess`, the ownership of the 
> FD is transfered into the subprocess.  If we plan to send the same FD to the 
> Executor and Fetcher, the fetcher cannot be allowed to take ownership.
> 
> 
> https://github.com/apache/mesos/blob/86c9f063b8d94a5e1bd611db5850d9d31dbb30b8/include/mesos/slave/container_logger.hpp#L79-L86
> 
> Jiang Yan Xu wrote:
> Thanks for the reply. In addition to passing SubprocessInfo to the 
> fetcher, we have to set the perms for the files. Since it's only relavent to 
> the sandbox logger, I guess we have to handle it in the sandbox logger 
> itself? This would probably requ

Re: Review Request 45962: Added a persistent volume test framework for shared volumes.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 11:49 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Added MESOS_RUNTIME_DIR to point to an existing directory for this test 
framework to run.


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


Repository: mesos


Description
---

Added a persistent volume test framework for shared volumes.


Diffs (updated)
-

  src/Makefile.am 410b5f2e5bc50a5aa7856645c8cf4a3e43505a84 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/tests/examples_tests.cpp 52fac33733cc86dd718c7836c5031999f1597a5c 
  src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 

Diff: https://reviews.apache.org/r/45962/diff/


Testing
---

New test framework for shared resources added.
Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 52284: Implement more quota validation tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52103, 52284]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 26, 2016, 9:58 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52284/
> ---
> 
> (Updated Sept. 26, 2016, 9:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4941
> https://issues.apache.org/jira/browse/MESOS-4941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement more quota validation tests.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/52284/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 45967: Added documentation for shareable resources.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 11:50 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added documentation for shareable resources.


Diffs (updated)
-

  docs/home.md ad59eb1722b49cd72454795c095ddbd1ec1eb4dd 
  docs/shared-resources.md PRE-CREATION 

Diff: https://reviews.apache.org/r/45967/diff/


Testing
---

Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45963: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 11:50 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Allow the task to specify the persistent volume access to be read-only
or read-write. Note that the persistent volume is always created as
read-write.
If the task is the first consumer of the shared persistent volume, then
set the ownership of the persistent volume to match that of the task.
Otherwise, allow the task to be executed only if the ownership of the
persistent volume matches that of the task.
Added an option to run the test in mixed (default) mode or shared-only
mode. In mixed mode, multiple shards alternate between shared and
unshared persistent volumes for the tasks. In shared-only mode, all
shards use shared persistent volumes for the tasks.


Diffs (updated)
-

  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
  src/master/validation.cpp aa743d730bebebc0353fa0a7a31f68bc94e7e25d 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
0a85935550e36c9142d845465cfa70a1634a647a 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
ea418252956c8089acc5a491888ed7f6df6cafcd 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
794b6e5990db5f8eb21a6535872f284ca02e0553 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
af427c6e5691f1770ab3ebef79502eb2c2176c4a 

Diff: https://reviews.apache.org/r/45963/diff/


Testing
---

Tests successful.


Thanks,

Anindya Sinha

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

[Anindya Sinha]

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

(Updated Sept. 26, 2016, 11:50 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Allocations test has the following resource configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.

This test is parameterized based on number of agents, number of
frameworks and resource configuration.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
46553f6d501deb37862dd5ba48be1c114f6e6cb8 

Diff: https://reviews.apache.org/r/49571/diff/


Testing
---

All tests passed.

Allocations benchmark test results
==
Support of shared resources has a small impact (roughly 10%) on runtime 
performance in allocations as compared to HEAD (without shared resources). 
Also, there is no visible impact in performance when shared resources are added 
in the tests.

Following is a snapshot with 1000 agents and 200 frameworks.

With the patch (and no shared resources)

[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6907us
Added 1000 agents in 2.057098secs
round 0 allocate() took 1.689164secs to make 1000 offers
round 50 allocate() took 1.672373secs to make 1000 offers
round 100 allocate() took 1.680571secs to make 1000 offers
round 150 allocate() took 1.674683secs to make 1000 offers
round 199 allocate() took 1.671525secs to make 1000 offers

With the patch (and shared resources on all agents)
---
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
Using 1000 agents and 200 frameworks with resource type 1
Added 200 frameworks in 6888us
Added 1000 agents in 2.096218secs
round 0 allocate() took 1.704491secs to make 1000 offers
round 50 allocate() took 1.718623secs to make 1000 offers
round 100 allocate() took 1.716224secs to make 1000 offers
round 150 allocate() took 1.707343secs to make 1000 offers
round 199 allocate() took 1.727467secs to make 1000 offers

With the patch (and shared resources on alternate agents)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
Using 1000 agents and 200 frameworks with resource type 2
Added 200 frameworks in 7304us
Added 1000 agents in 2.071009secs
round 0 allocate() took 1.689045secs to make 1000 offers
round 50 allocate() took 1.691524secs to make 1000 offers
round 100 allocate() took 1.688873secs to make 1000 offers
round 150 allocate() took 1.688713secs to make 1000 offers
round 199 allocate() took 1.691223secs to make 1000 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6801us
Added 1000 agents in 1.721447secs
round 0 allocate() took 1.502953secs to make 1000 offers
round 50 allocate() took 1.520157secs to make 1000 offers
round 100 allocate() took 1.517221secs to make 1000 offers
round 150 allocate() took 1.526446secs to make 1000 offers
round 199 allocate() took 1.538005secs to make 1000 offers


Thanks,

Anindya Sinha

Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Anindya Sinha]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

When a framework issues a DESTROY of a shared volume, and that volume
is not in use by a running or a pending task, we rescind the offers
from frameworks to which the shared volume is present in pending offers
so that the volume is not assigned to any task in a future ACCEPT call.
At that time, we need to recover the resources as well for proper
accounting of such resources by the allocator.


Diffs
-

  src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 
  src/tests/persistent_volume_tests.cpp 
2ab44d11d159162dfcac9d0791b651ed059b8164 

Diff: https://reviews.apache.org/r/52288/diff/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 52290: Removed resources field from LaunchNestedContainer agent::Call.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This field is currently not used, in that nested containers will
share the resources allocated to the parent container, regardless
of whether resources are specified.

In the future when we introduce the ability to provide nested
container resource allocation and isolation, we can re-introduce
the field as necessary.


Diffs
-

  include/mesos/agent/agent.proto 3397c4177813252b777bbe328ff92c9428366b70 
  include/mesos/v1/agent/agent.proto 93c817c0be92f68dea28a9272ac034386ade02af 
  src/slave/containerizer/containerizer.hpp 
47a8582f37c0fc9fb0d8cb8a0b979802f603b25f 
  src/slave/containerizer/mesos/containerizer.hpp 
cf0ec671c1af7450c2eb44a899c98dbf403a76b1 
  src/slave/containerizer/mesos/containerizer.cpp 
a88e4039da7319bc5b6c4003b67ef7c23ad5c320 
  src/slave/http.cpp 349d307284809353d950c6f281aa8605eb9289e9 
  src/tests/api_tests.cpp e857b17cfe5f05d59859263c025564d33700a26c 
  src/tests/containerizer/mock_containerizer.hpp 
840c51148399d2d2ebfb59106c1732ed5e91e61e 

Diff: https://reviews.apache.org/r/52290/diff/


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 52234: Added containerizer helper method 'getContainerIds()'.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

This is a helper function to collect all containerIds (include top
level and nested) from the runtime directory.


Diffs (updated)
-

  src/slave/containerizer/mesos/paths.hpp 
c47fd8c1743eb461a6774b4b6aa646d847c4efe0 
  src/slave/containerizer/mesos/paths.cpp 
faa588a06c8cceec88eaa132e44e30040fb95478 

Diff: https://reviews.apache.org/r/52234/diff/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 52241: Removed unit test 'IsolatorCleanupBeforePrepare'.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

This is a regression test for a bug which was already fixed in our code.
The bug was that we missed a check in containerizer::recover() to make
sure we will return a failure if the container state is changed to
'DESTROYING'. Otherwise, it was possible that the container state
might be reset from 'DESTORYING' to 'PREPARING' by the race between
containerizer::prepare and containerizer::destory.

However, while this regression test tries to reproduce the race and
tests on that the issue is fixed. It was making the assumption that
if the container is in PROVISIONING state and we call
containerizer::destroy(), we will always reach the future of
'container->provisioning' and waiting for the call back. This is
not true, because it is possible that when the call back is triggered
and we call containerizer::prepare(), the destroy track does not
reach the 'container->provisioning' yet. The race exists in the unit
test for a while.

Now, after we implemented the code for nested container support, the
containerizer::destroy become slower because we changed it to be
recursive (destroy child containers first before destroying the parent
container). Then, it exposes the race in this unit test, since when
the callback is triggered and containerizer call prepare(), the
containerizer::destroy() does not change the container state to
'DESTROYING' yet.

After some intestigation, because the containerizer return type is
void and all its continuous method (e.g., __destroy) only take one
parameter 'ContainerID' and there is no special variable we can
inject in to the method, there is no technical tool for us to make
sure the destroy track reachs 'container->provisioning' point and
waiting for a callback.

After a second thought, the implementation in the containerizer to
deal with container state is handled correctly. We should remove
this regression test if such a race is not fixable.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
8b5a19e121ef74eaf99b39682f8fd170605bf56d 

Diff: https://reviews.apache.org/r/52241/diff/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 51825: Updated the streaming function for ContainerID to be nesting aware.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:12 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

We are using '.' as a separator. If we have a nested container "yyy"
under its parent container "xxx". Then, the streaming function will
print it out as "xxx.yyy".


Diffs (updated)
-

  src/common/type_utils.cpp 9cb6274231bc373e796dc8e91e9340025b57ad1e 

Diff: https://reviews.apache.org/r/51825/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52008: Added slave helper function for nested containers 'getSandboxPath()'.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Added slave helper function for nested containers 'getSandboxPath()'.


Diffs (updated)
-

  src/slave/containerizer/mesos/paths.hpp 
c47fd8c1743eb461a6774b4b6aa646d847c4efe0 
  src/slave/containerizer/mesos/paths.cpp 
faa588a06c8cceec88eaa132e44e30040fb95478 

Diff: https://reviews.apache.org/r/52008/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51673: Updated mesos containerizer isolate and status for nested support.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


Repository: mesos


Description
---

This patch simply removed the check.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
a88e4039da7319bc5b6c4003b67ef7c23ad5c320 

Diff: https://reviews.apache.org/r/51673/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52233: Changed 'ExecutorInfo' to be optional in ContainerState.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

This is necessary for recovering nested containers, because a list of
'ContainerState' including recoverable top level containers and nested
containers will be passed to isolator::recover, but there is no way for
nested containers to carry the ExecutorInfo.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/common/protobuf_utils.hpp c5e5a9ad10134bcf87b59460bec7152e4916c75d 
  src/common/protobuf_utils.cpp 5db4be4bdc7b9a3a2a66a17f8a9ac74c8d3dfbf6 

Diff: https://reviews.apache.org/r/52233/diff/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 52003: Supported volume/image isolator to be nested aware.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Supported volume/image isolator to be nested aware.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/volume/image.hpp 
6333e9c881b10184fac2f15f5f4a6f7d781a655f 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
c25205bb80d8008e7879c7a9e6fc274ea0cee5f3 

Diff: https://reviews.apache.org/r/52003/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52004: Supported docker runtime isolator to be nested aware.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Supported docker runtime isolator to be nested aware.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
ac0397f65bb2a675ad1eae0f7cfa95f10145fc63 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
b589cd691ae6aacd2dcd00878e43d58f15abfe11 

Diff: https://reviews.apache.org/r/52004/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51669: Changed ContainerConfig::ExecutorInfo from required to optional.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:12 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


Repository: mesos


Description
---

For nested container launch in containerizer, a new launch method
was introduced for nested container only. However, 'ExecutorInfo'
is not included in this new launch method, only 'CommandInfo'
instead. To construct the 'ContainerConfig' for nested container
launch, we need to update the 'ExecutorInfo' in 'ContainerConfig'
from required to optional.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 

Diff: https://reviews.apache.org/r/51669/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52005: Supported appc/runtime isolator to be nested aware.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Supported appc/runtime isolator to be nested aware.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/appc/runtime.hpp 
c25b0eeff44d66c2045fd8daf0feb8ea3db718e4 
  src/slave/containerizer/mesos/isolators/appc/runtime.cpp 
a96298b801e8ef217dc0e88187b527e3c43a338e 

Diff: https://reviews.apache.org/r/52005/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51674: Supported mesos containerizer destroy to be nested aware.

[Gilbert Song]

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

(Updated Sept. 26, 2016, 5:13 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


Repository: mesos


Description
---

This patch makes the mesos containerizer destroy to be recursive.
To destroy a container with children nested under it, all its
children will be destroyed first, recursively. Then, the container
will get destroyed after all children destroy finish.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cf0ec671c1af7450c2eb44a899c98dbf403a76b1 
  src/slave/containerizer/mesos/containerizer.cpp 
a88e4039da7319bc5b6c4003b67ef7c23ad5c320 

Diff: https://reviews.apache.org/r/51674/diff/


Testing
---

make check


Thanks,

Gilbert Song