Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-14 Thread Alexander Rukletsov 

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


Ship it!




Ship It!

- Alexander Rukletsov


On June 13, 2018, 9:23 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 13, 2018, 9:23 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable was initialized using a dynamic memory
> allocation. This is against our coding style, and makes it
> as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-13 Thread Mesos Reviewbot 

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On June 13, 2018, 9:23 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 13, 2018, 9:23 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable was initialized using a dynamic memory
> allocation. This is against our coding style, and makes it
> as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-13 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['67525']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (127 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1131 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (36 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (40 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (80 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (851 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (875 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (843 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (867 ms total)

[--] Global test environment tear-down
[==] 988 tests from 97 test cases ran. (496495 ms total)
[  PASSED  ] 987 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 220 DISABLED TESTS

```

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

```
I0613 11:51:07.106554   716 slave.cpp:3939] Shutting down framework 
91665eeb-576a-4d3e-94f9-647cc079b9d8-
I0613 11:51:07.106554  1328 master.cpp:10863] Updating the state of task 
9114be25-ec5c-4382-825a-06f69637a116 of framework 
91665eeb-576a-4d3e-94f9-647cc079b9d8- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0613 11:51:07.106554   716 slave.cpp:6660] Shutting down executor 
'9114be25-ec5c-4382-825a-06f69637a116' of framework 
91665eeb-576a-4d3e-94f9-647cc079b9d8- at executor(1)@192.10.1.5:52339
I0613 11:51:07.108556   716 slave.cpp:931] Agent terminating
W0613 11:51:07.109555   716 slave.cppI0613 11:51:06.933574   604 exec.cpp:162] 
Version: 1.7.0
I0613 11:51:06.961549  2352 exec.cpp:236] Executor registered on agent 
91665eeb-576a-4d3e-94f9-647cc079b9d8-S0
I0613 11:51:06.965580  2124 executor.cpp:178] Received SUBSCRIBED event
I0613 11:51:06.970556  2124 executor.cpp:182] Subscribed executor on 
windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net
I0613 11:51:06.971556  2124 executor.cpp:178] Received LAUNCH event
I0613 11:51:06.976554  2124 executor.cpp:665] Starting task 
9114be25-ec5c-4382-825a-06f69637a116
I0613 11:51:07.062561  2124 executor.cpp:485] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0613 11:51:07.076561  2124 executor.cpp:678] Forked command at 7056
I0613 11:51:07.109555  7648 exec.cpp:445] Executor asked to shutdown
I0613 11:51:07.110556  2124 executor.cpp:178] Received SHUTDOWN event
I0613 11:51:07.110556  2124 executor.cpp:781] Shutting down
I0613 11:51:07.110556  2124 executor.cpp:894] Sending SIGTERM to process tree 
at pid 705:3935] Ignoring shutdown framework 
91665eeb-576a-4d3e-94f9-647cc079b9d8- because it is terminating
I0613 11:51:07.110556  1328 master.cpp:10962] Removing task 
9114be25-ec5c-4382-825a-06f69637a116 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 91665eeb-576a-4d3e-94f9-647cc079b9d8- on 
agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613 11:51:07.113554  5536 master.cpp:1293] Agent 
91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net) disconnected
I0613 11:51:07.113554  5536 master.cpp:3303] Disconnecting agent 
91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613 11:51:07.114547  5536 master.cpp:3322] Deactivating agent 
91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-13 Thread Benno Evers 

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

(Updated June 13, 2018, 9:23 a.m.)


Review request for mesos.


Changes
---

Added tests.


Repository: mesos


Description (updated)
---

The static variable was initialized using a dynamic memory
allocation. This is against our coding style, and makes it
as easy as writing

static process::http::NotFound DEFAULT_RESPONSE;

for a user to introduce a potential segfault into his code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
055447e13117c0a3ba79d0fc326ece657e8f064f 
  3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658 
  3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
  3rdparty/libprocess/src/tests/http_tests.cpp 
ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d 


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

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


Testing
---

`make check`


Thanks,

Benno Evers

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Mesos Reviewbot 

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 67525 was successfully built and tested.

Reviews applied: `['67525']`

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

- Mesos Reviewbot Windows


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Benno Evers 

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




3rdparty/libprocess/include/process/http.hpp
Line 121 (original)


I put this in the review, but actually I'm not sure if this is ok regarding 
our rules concerning API and ABI stability.

If desired, I  can change to one of these alternatives:

1) Keep the hashmap in the public API, mark it as deprecated
2) Remove hashmap from the public API, but still define it in `http.cpp` to 
keep ABI compatibility;


- Benno Evers


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Benno Evers 

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

Review request for mesos.


Repository: mesos


Description
---

The static variable `http::statuses` was initialized using a
dynamic memory allocation. This is against our coding style,
and makes it as easy as writing

static process::http::NotFound DEFAULT_RESPONSE;

for a user to introduce a potential segfault into his code.

Therefore, it was changed to use an array with constant
initialization instead.

The pointer `statuses` was also removed from the public API, since
there are no current users outside libprocess, providing a non-const
pointer to an internal data structure seems inherently unsafe, and
the intended use-cases seem to be covered by the existing `Status::string()`
function.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
055447e13117c0a3ba79d0fc326ece657e8f064f 
  3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658 
  3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 


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


Testing
---

`make check`


Thanks,

Benno Evers