You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2017/12/20 00:51:31 UTC

Review Request 64739: Upgraded resources that come from `protobuf::read`.

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/resource_provider/storage/provider.cpp 79d7f602bbc57bce308157dfbb6c2afaef806f23 
  src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 


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


Testing
-------


Thanks,

Michael Park


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/#review194707
-----------------------------------------------------------




src/resource_provider/storage/provider.cpp
Lines 921 (patched)
<https://reviews.apache.org/r/64739/#comment273642>

    `resourceProviderState.get()` is const.


- Chun-Hung Hsiao


On Jan. 2, 2018, 11:39 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2018, 11:39 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8221
>     https://issues.apache.org/jira/browse/MESOS-8221
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Upgraded resources that come from `protobuf::read`.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp ee9ac901a6f46b6c6749381d859b3b090c113673 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/
-----------------------------------------------------------

(Updated Jan. 2, 2018, 3:39 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Rebased.


Repository: mesos


Description (updated)
-------

Upgraded resources that come from `protobuf::read`.


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp ee9ac901a6f46b6c6749381d859b3b090c113673 
  src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 


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

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


Testing
-------


Thanks,

Michael Park


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Michael Park <mp...@apache.org>.

> On Dec. 21, 2017, 6:55 p.m., Benjamin Mahler wrote:
> > Hm.. I was a little puzzled here, why do some use convert with POST and some use the reflection based upgrade? Shouldn't we just use the reflection based upgrade everywhere?

Sorry for the confusion. The `upgradeResources` used here is the one pulled out of
`validateAndUpgradeResources`, and is not "reflection based upgrade". I called
it `upgradeResources` for now in the hopes that when we do introduce reflection-based
`upgradeResources`, the callsite wouldn't need to change. Although looking at it now
we'll probably be invoking the reflection-based upgrade on all of `resourceProviderState`
i.e., `upgradeResources(resourceProviderState.get());`


- Michael


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


On Dec. 19, 2017, 4:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2017, 4:51 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp 79d7f602bbc57bce308157dfbb6c2afaef806f23 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/#review194396
-----------------------------------------------------------



Hm.. I was a little puzzled here, why do some use convert with POST and some use the reflection based upgrade? Shouldn't we just use the reflection based upgrade everywhere?

- Benjamin Mahler


On Dec. 20, 2017, 12:51 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp 79d7f602bbc57bce308157dfbb6c2afaef806f23 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Michael Park <mp...@apache.org>.

> On Jan. 2, 2018, 2:39 p.m., Benjamin Mahler wrote:
> > Is there a ticket for being more robust in the face of inability to recover a downgraded agent? Can you file one if not?
> > Also, do we want to target a ticket for 2.1 that stops downgrading? I assume the strategy to end the 1.x deprecation cycle will be that 2.1+ only supports being upgraded from 2.0.x.

Found the "downgrade capability" ticket: https://issues.apache.org/jira/browse/MESOS-7682


- Michael


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


On Jan. 2, 2018, 3:39 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2018, 3:39 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Upgraded resources that come from `protobuf::read`.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp ee9ac901a6f46b6c6749381d859b3b090c113673 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/#review194659
-----------------------------------------------------------


Ship it!




Is there a ticket for being more robust in the face of inability to recover a downgraded agent? Can you file one if not?
Also, do we want to target a ticket for 2.1 that stops downgrading? I assume the strategy to end the 1.x deprecation cycle will be that 2.1+ only supports being upgraded from 2.0.x.


src/resource_provider/storage/provider.cpp
Line 663 (original), 663-664 (patched)
<https://reviews.apache.org/r/64739/#comment273578>

    Do you want a TODO here to use a reflection based approach?



src/resource_provider/storage/provider.cpp
Lines 854-862 (patched)
<https://reviews.apache.org/r/64739/#comment273579>

    Ditto here



src/slave/containerizer/mesos/paths.cpp
Lines 288-289 (patched)
<https://reviews.apache.org/r/64739/#comment273580>

    Ditto here



src/slave/containerizer/mesos/paths.cpp
Lines 341-350 (patched)
<https://reviews.apache.org/r/64739/#comment273581>

    Ditto here.


- Benjamin Mahler


On Dec. 20, 2017, 12:51 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp 79d7f602bbc57bce308157dfbb6c2afaef806f23 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 64739: Upgraded resources that come from `protobuf::read`.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64739/#review194199
-----------------------------------------------------------



FAIL: Some Mesos tests failed.

Reviews applied: `['64737', '64738', '63976', '63977', '64739']`

Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`

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

Relevant logs:

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

```
[----------] 9 tests from Endpoint/SlaveEndpointTest (948 ms total)

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

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

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

[----------] Global test environment tear-down
[==========] 845 tests from 85 test cases ran. (299180 ms total)
[  PASSED  ] 843 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ResourceFormatTest.DowngradeWithResourcesWithoutRefinedReservations
[  FAILED  ] ResourceFormatTest.DowngradeWithResourcesWithRefinedReservations

 2 FAILED TESTS
  YOU HAVE 213 DISABLED TESTS

```

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

```
I1220 01:58:37.800837  5172 master.cpp:10141] Updating the state of task cd39a23c-0cbc-45e2-9860-1d3cd5ff8b54 of framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I1220 01:58:37.800837  9044 slave.cpp:3392] Shutting down framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000
I1220 01:58:37.800837  9044 slave.cpp:6083] Shutting down executor 'cd39a23c-0cbc-45e2-9860-1d3cd5ff8b54' of framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000 at executor(1)@10.3.1.5:I1220 01:58:37.159876  3568 exec.cpp:162] Version: 1.5.0
I1220 01:58:37.181856  3820 exec.cpp:237] Executor registered on agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0
I1220 01:58:37.184855  6504 executor.cpp:171] Received SUBSCRIBED event
I1220 01:58:37.188877  6504 executor.cpp:175] Subscribed executor on build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I1220 01:58:37.188877  6504 executor.cpp:171] Received LAUNCH event
I1220 01:58:37.192831  6504 executor.cpp:638] Starting task cd39a23c-0cbc-45e2-9860-1d3cd5ff8b54
I1220 01:58:37.266876  6504 executor.cpp:478] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I1220 01:58:37.778834  6504 executor.cpp:651] Forked command at 6856
I1220 01:58:37.802814  7272 exec.cpp:435] Executor asked to shutdown
I1220 01:58:37.803814  7400 executor.cpp:171] Received SHUTDOWN event
I1220 01:58:37.803814  7400 executor.cpp:748] Shutting down
I1220 01:58:37.803814  7400 executor.cpp:855] Sending SIGTERM to process tree at pid 653846
I1220 01:58:37.801834  9044 slave.cpp:931] Agent terminating
W1220 01:58:37.801834  9044 slave.cpp:3388] Ignoring shutdown framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000 because it is terminating
I1220 01:58:37.802814  5172 master.cpp:10247] Removing task cd39a23c-0cbc-45e2-9860-1d3cd5ff8b54 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000 on agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0 at slave(327)@10.3.1.5:53825 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1220 01:58:37.805814  5200 containerizer.cpp:2352] Destroying container 9a438440-edb7-4fdb-9aef-c7d43254ed02 in RUNNING state
I1220 01:58:37.805814  5200 containerizer.cpp:2966] Transitioning the state of container 9a438440-edb7-4fdb-9aef-c7d43254ed02 from RUNNING to DESTROYING
I1220 01:58:37.805814  5172 master.cpp:1305] Agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0 at slave(327)@10.3.1.5:53825 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I1220 01:58:37.805814  1696 hierarchical.cpp:344] Removed framework 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-0000
I1220 01:58:37.805814  5172 master.cpp:3364] Disconnecting agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0 at slave(327)@10.3.1.5:53825 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1220 01:58:37.805814  5172 master.cpp:3383] Deactivating agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0 at slave(327)@10.3.1.5:53825 (build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1220 01:58:37.806814  5200 launcher.cpp:156] Asked to destroy container 9a438440-edb7-4fdb-9aef-c7d43254ed02
I1220 01:58:37.806814  2280 hierarchical.cpp:766] Agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0 deactivated
I1220 01:58:37.867903  2280 containerizer.cpp:2805] Container 9a438440-edb7-4fdb-9aef-c7d43254ed02 has exited
I1220 01:58:37.895931  3948 master.cpp:1147] Master terminating
I1220 01:58:37.897913  1696 hierarchical.cpp:609] Removed agent 8c74b472-e0a3-4fd1-bdc5-4060860e1e40-S0
I1220 01:58:38.240924  8060 process.cpp:887] Failed to accept socket: future discarded
```

- Mesos Reviewbot Windows


On Dec. 20, 2017, 12:51 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64739/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp 79d7f602bbc57bce308157dfbb6c2afaef806f23 
>   src/slave/containerizer/mesos/paths.cpp 8a188a918873eef468a984b80f5ea7ebaa8fb923 
> 
> 
> Diff: https://reviews.apache.org/r/64739/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>