Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-15 Thread Eric Chung 

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


Ship it!




Ship It!

- Eric Chung


On March 15, 2018, 8:18 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 15, 2018, 8:18 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
> and replaced with those in a `MesosContainerLauncherHelper` subclass
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 75b7eaf9cd62d6b5f02896175168b651f4517e12 
> 
> 
> Diff: https://reviews.apache.org/r/65899/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-15 Thread Jason Lai 

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

(Updated March 15, 2018, 8:18 a.m.)


Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James 
Peach, and Zhitao Li.


Changes
---

Use unique pointer over raw pointer of `MesosContainerizerLaunchHelper` 
instance.


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


Repository: mesos


Description
---

Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
and replaced with those in a `MesosContainerLauncherHelper` subclass
instead.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 
75b7eaf9cd62d6b5f02896175168b651f4517e12 


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

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


Testing
---


Thanks,

Jason Lai

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-14 Thread Jason Lai 


> On March 7, 2018, 2:28 a.m., Eric Chung wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 583 (patched)
> > 
> >
> > why is the explicit delete needed here? was it not being cleaned up 
> > previously?

A `MesosContainerizerLauncherHelper` instance was initiated as a raw pointer to 
the heap. So the intention was that the receiver takes care of managing the 
instance's lifecycle.

However, I think that we can have something smarter like wrapping the raw 
pointer with libprocess' `Owned` as a uniquely owned pointer. Will do that with 
a revision.


- Jason


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


On March 5, 2018, 7:31 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 5, 2018, 7:31 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
> and replaced with those in a `MesosContainerLauncherHelper` subclass
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 75b7eaf9cd62d6b5f02896175168b651f4517e12 
> 
> 
> Diff: https://reviews.apache.org/r/65899/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-06 Thread Eric Chung 

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




src/slave/containerizer/mesos/launch.cpp
Lines 583 (patched)


why is the explicit delete needed here? was it not being cleaned up 
previously?


- Eric Chung


On March 5, 2018, 7:31 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 5, 2018, 7:31 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
> and replaced with those in a `MesosContainerLauncherHelper` subclass
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 75b7eaf9cd62d6b5f02896175168b651f4517e12 
> 
> 
> Diff: https://reviews.apache.org/r/65899/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-05 Thread Jason Lai 


> On March 5, 2018, 9:10 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['65811', '65812', '65898', '65899']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65899
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65899/logs/mesos-tests-cmake-stdout.log):
> > 
> > ```
> >  
> > D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(44): 
> > warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible 
> > loss of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  
> > D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(45): 
> > warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible 
> > loss of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(95): warning C4267: '=': 
> > conversion from 'size_t' to 'jint', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(463): warning C4244: 
> > 'initializing': conversion from 'jchar' to 'char', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(493): warning C4244: 
> > 'initializing': conversion from 'jlong' to 'long', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(654): warning C4244: 
> > 'initializing': conversion from 'jchar' to 'char', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(693): warning C4244: 
> > 'initializing': conversion from 'jlong' to 'long', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(770): warning C4244: 
> > 'initializing': conversion from 'jchar' to 'char', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> >  D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(809): warning C4244: 
> > 'initializing': conversion from 'jlong' to 'long', possible loss of data 
> > [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
> >"D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (11) 
> > ->
> >
> > "D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj" 
> > (default target) (19) ->
> >(Link target) -> 
> >  mesos.lib(launch.obj) : error LNK2019: unresolved external symbol 
> > "public: static class Try > mesos::internal::slave::MesosContainerizerLaunchHelper *,class Error> 
> > __cdecl 
> > mesos::internal::slave::MesosContainerizerLaunchHelper::create(class 
> > mesos::slave::ContainerLaunchInfo const &)" 
> > (?create@MesosContainerizerLaunchHelper@slave@internal@mesos@@SA?AV?$Try@PEAVMesosContainerizerLaunchHelper@slave@internal@mesos@@VErrorAEBVContainerLaunchInfo@24@@Z)
> >  referenced in function "protected: virtual int __cdecl 
> > mesos::internal::slave::MesosContainerizerLaunch::execute(void)" 
> > (?execute@MesosContainerizerLaunch@slave@internal@mesos@@MEAAHXZ) 
> > [D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj]
> >  D:\DCOS\mesos\src\mesos-containerizer.exe : fatal error LNK1120: 1 
> > unresolved externals 
> > [D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
> >"D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (11) 
> > ->
> >"D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj" (default target) 
> > (29) ->
> >  mesos.lib(launch.obj) : error LNK2019: unresolved external symbol 
> > "public: static class Try > mesos::internal::slave::MesosContainerizerLaunchHelper *,class Error> 
> > __cdecl 
> > mesos::internal::slave::MesosContainerizerLaunchHelper::create(class 
> > mesos::slave::ContainerLaunchInfo const &)" 
> > (?create@MesosContainerizerLaunchHelper@slave@internal@mesos@@SA?AV?$Try@PEAVMesosContainerizerLaunchHelper@slave@internal@mesos@@VErrorAEBVContainerLaunchInfo@24@@Z)
> >  referenced in function "protected: virtual int __cdecl 
> > mesos::internal::slave::MesosContainerizerLaunch::execute(void)" 
> > (?execute@MesosContainerizerLaunch@slave@internal@mesos@@MEAAHXZ) 
> > [D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj]
> >  D:\DCOS\mesos\src\mesos-executor.exe : fatal error LNK1120: 1 
> > unresolved externals [D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj]
> > 
> > 214 Warning(s)
> > 4 Error(s)
> > 
> > Time Elapsed 00:23:35.74
> > ```

[65900](https://reviews.apache.org/r/65900/) has the

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-05 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [65811, 65812, 65898, 65899]

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

- Mesos Reviewbot


On March 5, 2018, 7:31 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 5, 2018, 7:31 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
> and replaced with those in a `MesosContainerLauncherHelper` subclass
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 75b7eaf9cd62d6b5f02896175168b651f4517e12 
> 
> 
> Diff: https://reviews.apache.org/r/65899/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-05 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65811', '65812', '65898', '65899']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65899

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65899/logs/mesos-tests-cmake-stdout.log):

```
 
D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(44): 
warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss 
of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 
D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(45): 
warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss 
of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(95): warning C4267: '=': 
conversion from 'size_t' to 'jint', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(463): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(493): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(654): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(693): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(770): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(809): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]


   "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (11) ->
   
"D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj" 
(default target) (19) ->
   (Link target) -> 
 mesos.lib(launch.obj) : error LNK2019: unresolved external symbol 
"public: static class Try __cdecl 
mesos::internal::slave::MesosContainerizerLaunchHelper::create(class 
mesos::slave::ContainerLaunchInfo const &)" 
(?create@MesosContainerizerLaunchHelper@slave@internal@mesos@@SA?AV?$Try@PEAVMesosContainerizerLaunchHelper@slave@internal@mesos@@VErrorAEBVContainerLaunchInfo@24@@Z)
 referenced in function "protected: virtual int __cdecl 
mesos::internal::slave::MesosContainerizerLaunch::execute(void)" 
(?execute@MesosContainerizerLaunch@slave@internal@mesos@@MEAAHXZ) 
[D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj]
 D:\DCOS\mesos\src\mesos-containerizer.exe : fatal error LNK1120: 1 
unresolved externals 
[D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj]


   "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (11) ->
   "D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj" (default target) 
(29) ->
 mesos.lib(launch.obj) : error LNK2019: unresolved external symbol 
"public: static class Try __cdecl 
mesos::internal::slave::MesosContainerizerLaunchHelper::create(class 
mesos::slave::ContainerLaunchInfo const &)" 
(?create@MesosContainerizerLaunchHelper@slave@internal@mesos@@SA?AV?$Try@PEAVMesosContainerizerLaunchHelper@slave@internal@mesos@@VErrorAEBVContainerLaunchInfo@24@@Z)
 referenced in function "protected: virtual int __cdecl 
mesos::internal::slave::MesosContainerizerLaunch::execute(void)" 
(?execute@MesosContainerizerLaunch@slave@internal@mesos@@MEAAHXZ) 
[D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj]
 D:\DCOS\mesos\src\mesos-executor.exe : fatal error LNK1120: 1 
unresolved externals [D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj]

214 Warning(s)
4 Error(s)

Time Elapsed 00:23:35.74
```

- Mesos Reviewbot Windows


On March 5, 2018, 7:31 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 5, 2018, 7:31 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung,

Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

2018-03-04 Thread Jason Lai 

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

Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James 
Peach, and Zhitao Li.


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


Repository: mesos


Description
---

Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
and replaced with those in a `MesosContainerLauncherHelper` subclass
instead.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
75b7eaf9cd62d6b5f02896175168b651f4517e12 


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


Testing
---


Thanks,

Jason Lai