Re: Review Request 63391: Fixed the flaky SlaveTest.HTTPSchedulerSlaveRestart.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63391]

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 Oct. 28, 2017, 1:37 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63391/
> ---
> 
> (Updated Oct. 28, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7972
> https://issues.apache.org/jira/browse/MESOS-7972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test was trying to prevent re-registration retries, but
> it resumed the clock too early! As a result, sometimes a second
> re-registration occurs and the master will send an
> UpdateFrameworkMessage that over-rules the one we're spoofing
> in the test. One example occurs in MESOS-7972, and the agent
> as a result replies directly to the scheduler since it sees
> the scheduler as having a pid.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 99b9c8222d0c15e42b2d8d6672bc197dcb37b359 
> 
> 
> Diff: https://reviews.apache.org/r/63391/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 61810: Added a function to apply offer operations.

[Chun-Hung Hsiao]

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




src/resource_provider/manager.cpp
Lines 315 (patched)


Currently `protobuf::getConsumedResources()` only accepts `CREATE_VOLUME`, 
`DESTROY_VOLUME`, `CREATE_BLOCK`, and `DESTROY_BLOCK`. I think Jie's idea is 
that we still pass `RESERVE`, `UNRESERVE`, `CREATE` and `DESTROY` into the 
resource provider and ask the resource provider to checkpoint the information 
there, and avoid another checkpoint in the agent to simplify reconciliation.



src/resource_provider/manager.cpp
Lines 319 (patched)


If we only handle volume and block creation and destruction here, this loop 
is not required since the operations contain only a single resource. If we want 
to also pass in reservation and persistent volume operations, do we require the 
framework to send individual operations for each resource provider, or do we 
want to accept one single `CREATE` that contains resources from multiple 
providers, and do the multiplexing here?

The publish function I'm working on takes the latter approach.


- Chun-Hung Hsiao


On Oct. 20, 2017, 12:51 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61810/
> ---
> 
> (Updated Oct. 20, 2017, 12:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7594
> https://issues.apache.org/jira/browse/MESOS-7594
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource provider manager provides an 'apply' function for offer
> operation affecting resource providers. The resource on which the
> operation should be applied contains a resource provider ID. This will
> be extracted and an event will be sent to the respective resource
> provider.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/tests/resource_provider_manager_tests.cpp 
> ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
> 
> 
> Diff: https://reviews.apache.org/r/61810/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 62502: Added events to publish and unpublish resources.

[Chun-Hung Hsiao]

> On Oct. 17, 2017, 8:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 57-58 (patched)
> > 
> >
> > These two are not no needed for now. Let's remove it for now.

Also, since we don't do unpublish, the current semantics of the `PUBLISH` event 
is that the agent notifies a resource provider about ALL resources that needs 
to be in the "published" state. That means the resources may comes from all 
frameworks and there is no single `framework_id` that we can assign to this 
event. If we want to keep it the same in the future, then we probably need to 
change the interface of the resource allocator to avoid passing in 
`framework_id`s.


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> ---
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
> https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 63391: Fixed the flaky SlaveTest.HTTPSchedulerSlaveRestart.

[Benjamin Mahler]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

This test was trying to prevent re-registration retries, but
it resumed the clock too early! As a result, sometimes a second
re-registration occurs and the master will send an
UpdateFrameworkMessage that over-rules the one we're spoofing
in the test. One example occurs in MESOS-7972, and the agent
as a result replies directly to the scheduler since it sees
the scheduler as having a pid.


Diffs
-

  src/tests/slave_tests.cpp 99b9c8222d0c15e42b2d8d6672bc197dcb37b359 


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


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler

Re: Review Request 63390: WIP: Add unit tests for storage local resource provider.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [63390, 63389, 63388, 63387, 63386, 63023, 63022, 63021, 
63019, 63385, 63060, 63377, 63018, 62637, 62636, 62762, 62635, 62633, 63376, 
63058, 63057, 63056, 63054, 62145, 62144, 62143, 62142, 60891, 60890, 60889, 
60888, 61806, 61805]

Failed command: python support/apply-reviews.py -n -r 63021

Error:
2017-10-28 00:57:36 URL:https://reviews.apache.org/r/63021/diff/raw/ 
[17757/17757] -> "63021.patch" [1]
error: patch failed: src/resource_provider/storage/provider.cpp:124
error: src/resource_provider/storage/provider.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19920/console

- Mesos Reviewbot


On Oct. 28, 2017, 12:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63390/
> ---
> 
> (Updated Oct. 28, 2017, 12:32 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Add unit tests for storage local resource provider.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
>   src/tests/resource_provider/storage_local_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63390/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 63390: WIP: Add unit tests for storage local resource provider.

[Chun-Hung Hsiao]

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

Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


Repository: mesos


Description
---

WIP: Add unit tests for storage local resource provider.


Diffs
-

  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/tests/resource_provider/storage_local_tests.cpp PRE-CREATION 


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


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 63389: Added a mock resource provider manager.

[Chun-Hung Hsiao]

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

Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a `ResourceProviderManager*` parameter to the slave
constructor, so we can pass in a mock resource provider manager. The
mock manager can be used to test against either the agent or the
resource provider.

The declaration of `ResourceProviderManagerProcess` is moved to a
separated `manager_process.hpp` header file so we can mock the internal
process.


Diffs
-

  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/local/local.cpp 99c1b087d75a1759ed61a4e6178e15919bee1e77 
  src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/manager_process.hpp PRE-CREATION 
  src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
  src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
  src/tests/cluster.cpp d657da6d84e5ea28e0dad180dce069de569c5d38 
  src/tests/mesos.hpp 4b61f2dbd67a5e8a65a6655519c8f03e5d6c6954 
  src/tests/mesos.cpp 9185b5bf2175be5b0f6b6a03a04e9e5445bf22fd 
  src/tests/mock_slave.hpp 57189cee20511d145ae6b47a4dc2c66a14638dad 
  src/tests/mock_slave.cpp db24f9e5b71f558d2f811f0da8a9cc9c7c2dd341 
  src/tests/resource_provider/mock_manager.hpp PRE-CREATION 
  src/tests/resource_provider/mock_manager.cpp PRE-CREATION 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Review Request 63386: Added utility functions to print types in resource provider events.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added utility functions to print types in resource provider events.


Diffs
-

  include/mesos/resource_provider/type_utils.hpp PRE-CREATION 
  include/mesos/type_utils.hpp 3bbc1fe5cb539baa138823c0240e47d3f4a1909e 
  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/common/type_utils.cpp e40d87c962e1a4ad25ec2368a904254cb28f2738 
  src/resource_provider/type_utils.cpp PRE-CREATION 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63023: WIP: Added a test CSI plugin.

[Chun-Hung Hsiao]

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

(Updated Oct. 28, 2017, 12:08 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Bug fixes.


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


Repository: mesos


Description
---

The test CSI plugin would just create subdirectories under its working
directories to mimic the behavior of creating volumes, then bind-mount
those volumes to mimic publish.


Diffs (updated)
-

  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/examples/test_csi_plugin.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/63023/diff/3/

Changes: https://reviews.apache.org/r/63023/diff/2-3/


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported resources from CSI plugins in storage local resource provider.

[Chun-Hung Hsiao]

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

(Updated Oct. 28, 2017, 12:07 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Partially refactored.


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


Repository: mesos


Description (updated)
---

The following lists the steps to import resources from a CSI plugin:
1. Launch the node plugin
  1.1 GetSupportedVersions
  1.2 GetPluginInfo
  1.3 ProbeNode
  1.4 GetNodeCapabilities
2. Launch the controller plugin
  2.1 GetSuportedVersions
  2.2 GetPluginInfo
  2.3 GetControllerCapabilities
3. GetCapacity
4. ListVolumes
5. Report to the resource provider through UPDATE_TOTAL_RESOURCES


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


Diff: https://reviews.apache.org/r/63022/diff/7/

Changes: https://reviews.apache.org/r/63022/diff/6-7/


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63021: Added functions to launch CSI plugin in storage local resource provider.

[Chun-Hung Hsiao]

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

(Updated Oct. 28, 2017, 12:03 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Moved CSI endpoint preparation to helper path functions.


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


Repository: mesos


Description (updated)
---

During initialization, the storage local resource provider first tries
to recover its ID of the last session through reading the actual path
linked by `work_dir/resource_providers///latest`, then
subscribe to the agent's resource provider manager. If this is a new
subscription, it will set up a new dir for CSI plugins to put their
socket files.

Once the CSI socket dir is set up, the storage local resource provider
can connect to a CSI plugin through the following procedure:
1. It first tries to connect to the existing socket file if there is
   one. On success, just return the connection.
2. On failure, kill the running plugin and remove the socket file.
3. Launch a container to run the plugin, and w for the socket file to
   appear, then connect to the socket file.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


Diff: https://reviews.apache.org/r/63021/diff/6/

Changes: https://reviews.apache.org/r/63021/diff/5-6/


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 63385: Added utility functions for CSI responses and volume ID and metadata.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added utility functions for CSI responses and volume ID and metadata.


Diffs
-

  src/csi/utils.hpp PRE-CREATION 
  src/csi/utils.cpp PRE-CREATION 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63060: Added utility functions and structures for CSI version and capabilities.

[Chun-Hung Hsiao]

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

(Updated Oct. 27, 2017, 11:58 p.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Removed the default clauses for switch statements on proto3 enums.


Summary (updated)
-

Added utility functions and structures for CSI version and capabilities.


Repository: mesos


Description (updated)
---

This patch adds some helper structures and functions for CSI protobuf.
The comparison and output operators for `csi::Version` are declared in
the `::csi` namespace so they can find it through ADL. Also, it exposes
`::csi` in `mesos::csi` in `spec.hpp` instead of `client.hpp`.


Diffs (updated)
-

  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/csi/client.hpp df674e17260519f983367f7c01f1057a75076238 
  src/csi/spec.hpp 60e40e05d81c9bf649459e29479838dd80daa3aa 
  src/csi/utils.hpp PRE-CREATION 
  src/csi/utils.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/63060/diff/3/

Changes: https://reviews.apache.org/r/63060/diff/2-3/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Review Request 63379: Set max-size for a book thumbnail on the site.

[Tomasz Janiszewski]

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

Review request for mesos and James Peach.


Repository: mesos


Description
---

There is an issue with books thumbnails.
Covers are loaded from external resources and
we do not control images sizes and ratios.
When one book has different ration than other
there might be a situation when there is not
enough space (vertically) to  place items
in a row, so they are located in next row
leaving a blank space.


Diffs
-

  site/source/assets/css/main.css d6ecb18f8c9ce785deee18cb9d131f021b6611f7 


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


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 63321: Added protobuf messages for V1 scheduler operation feedback.

[Greg Mann]

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

(Updated Oct. 27, 2017, 10:42 p.m.)


Review request for mesos, Gaston Kleiman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds new and updated protobuf messages to facilitate
offer operation status updates, as well as acknowledgement of
those updates and operation status reconciliation.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
f82f74d1c3767f97a1e6dad8acf4602f39e18380 
  include/mesos/v1/scheduler/scheduler.proto 
1fb0254103e5bb4f2cb98ca973e9f4e7b378 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Review Request 63377: Added filesystem layout for storage resource providers.

[Chun-Hung Hsiao]

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

Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


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


Repository: mesos


Description
---

A storage resource provider can now store the following persistent CSI
data in `/csi/resource_providers//`:

1. Mount points of CSI volumes: `volumes/`
2. States of CSI volumes: `states//state`
3. Directory to place CSI endpoints: `/tmp/mesos-csi-XX`, which
   would be symlinked by `plugins//endpoint`.


Diffs
-

  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
  src/resource_provider/storage/paths.hpp PRE-CREATION 
  src/resource_provider/storage/paths.cpp PRE-CREATION 
  src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63018: Added filesystem layout for local resource providers.

[Chun-Hung Hsiao]

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

(Updated Oct. 27, 2017, 10:07 p.m.)


Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Removed the cross-agent `resource_providers` directory.


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


Repository: mesos


Description (updated)
---

A local resource provide can store checkpoints, whose lifecycles should
be tied to the slave id, under
`/meta/slaves//resource_providers///
`.


Diffs (updated)
-

  src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
  src/resource_provider/local.hpp ebaa07d03ad77d516066ee2d4b60864be0611b5f 
  src/resource_provider/local.cpp ad98f333c5668ca81de6e7ed3fc8f59323b151da 
  src/resource_provider/storage/provider.hpp 
6de88c2329b358fcf48bc39ddda0132170991c3c 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 
  src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 
  src/slave/paths.cpp fd546525b900cb6524fb9196d19616ef18de0f30 


Diff: https://reviews.apache.org/r/63018/diff/3/

Changes: https://reviews.apache.org/r/63018/diff/2-3/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 62636: Generated authentication tokens for local resource providers.

[Chun-Hung Hsiao]

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

(Updated Oct. 27, 2017, 9:58 p.m.)


Review request for mesos, Alexander Rojas, Greg Mann, Jie Yu, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

`LocalResourceProviderDaemon` now uses `Slave::secretGenerater` to
generate an authentication token for each local resource provider. The
authentication token can then be used to call the V1 agent API. In order
to generate the tokens, `LocalResourceProviderDaemon::load()` is now an
asynchronous function.


Diffs (updated)
-

  src/resource_provider/daemon.hpp ef6c356cb6ddb2594d767d7dd6052e9fd8df8263 
  src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
  src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 


Diff: https://reviews.apache.org/r/62636/diff/4/

Changes: https://reviews.apache.org/r/62636/diff/3-4/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 62633: Added authentication tokens for local resource providers.

[Chun-Hung Hsiao]

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

(Updated Oct. 27, 2017, 9:54 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

The authentication tokens could be used by the local resource poviders
to call V1 API.


Diffs
-

  src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
  src/resource_provider/local.hpp ebaa07d03ad77d516066ee2d4b60864be0611b5f 
  src/resource_provider/local.cpp ad98f333c5668ca81de6e7ed3fc8f59323b151da 
  src/resource_provider/storage/provider.hpp 
6de88c2329b358fcf48bc39ddda0132170991c3c 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Review Request 63376: Started `LocalResourceProviderDaemon` after obtaining the slave ID.

[Chun-Hung Hsiao]

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

Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


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


Repository: mesos


Description
---

The local resource provider daemon is now started aftor we get a slave
id (either through registration or recovery). The slave id will be
eventually passed into each local resource provider for checkpointing.


Diffs
-

  src/resource_provider/daemon.hpp ef6c356cb6ddb2594d767d7dd6052e9fd8df8263 
  src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
  src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 


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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63094: Added resource version uuid for offer operations.

[Greg Mann]

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


Fix it, then Ship it!





include/mesos/resource_provider/resource_provider.proto
Lines 58 (patched)


s/changes/change/

Here and elsewhere (also in the description :)



src/messages/messages.proto
Lines 716 (patched)


Maybe use `ResourceVersionUUID` here instead?


- Greg Mann


On Oct. 27, 2017, 5:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> ---
> 
> (Updated Oct. 27, 2017, 5:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp 
> ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Greg Mann]

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


Fix it, then Ship it!





include/mesos/resource_provider/resource_provider.proto
Lines 52 (patched)


s/framework specified/framework-specified/

s/id/ID/



include/mesos/resource_provider/resource_provider.proto
Lines 99 (patched)


s/framework specified/framework-specified/

s/id/ID/



src/messages/messages.proto
Line 610 (original), 610 (patched)


s/RESORUCE/RESOURCE/



src/messages/messages.proto
Lines 658 (patched)


s/framework specified/framework-specified/

s/id/ID/



src/messages/messages.proto
Lines 675 (patched)


s/framework specified/framework-specified/

s/id/ID/


- Greg Mann


On Oct. 27, 2017, 9:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> ---
> 
> (Updated Oct. 27, 2017, 9:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf definitions related to offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63001/diff/11/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63018: Added filesystem layout for local resource providers.

[Chun-Hung Hsiao]

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




src/slave/paths.hpp
Lines 55 (patched)


Working on a patch that removes this `resource_providers` directory. The 
fact that there're two `resource_providers` directories in this file, one lives 
across agents but the other doesn't, are confusing.


- Chun-Hung Hsiao


On Oct. 20, 2017, 8:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63018/
> ---
> 
> (Updated Oct. 20, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8097
> https://issues.apache.org/jira/browse/MESOS-8097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A local resource provide can use
> `work_dir/resource_providers///` to store whatever data
> whose lifecycle is not tied to the agent ID, and store checkpoints,
> whose lifecycles should be tied to the agent ID, under
> `work_dir/meta/slaves/latest/resource_providers///`.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
>   src/resource_provider/local.hpp ebaa07d03ad77d516066ee2d4b60864be0611b5f 
>   src/resource_provider/local.cpp ad98f333c5668ca81de6e7ed3fc8f59323b151da 
>   src/resource_provider/storage/provider.hpp 
> 6de88c2329b358fcf48bc39ddda0132170991c3c 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 
>   src/slave/paths.cpp fd546525b900cb6524fb9196d19616ef18de0f30 
> 
> 
> Diff: https://reviews.apache.org/r/63018/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Jie Yu]

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

(Updated Oct. 27, 2017, 9:04 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
Greg Mann, Jan Schlicht, and Vinod Kone.


Changes
---

Addressed comments from Chun.


Repository: mesos


Description
---

Updated protobuf definitions related to offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63001/diff/11/

Changes: https://reviews.apache.org/r/63001/diff/10-11/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Jie Yu]

> On Oct. 26, 2017, 10:49 p.m., Greg Mann wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Line 66 (original), 72 (patched)
> > 
> >
> > Do you want to use `OFFER_OPERATION` here instead? Or, do you think 
> > it's OK to just use `OPERATION` since this is within the RP? Here and 
> > elsewhere.

Yeah, I'll use OFFER_OPERATION consistently.


> On Oct. 26, 2017, 10:49 p.m., Greg Mann wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2145 (patched)
> > 
> >
> > I'm a little bit concerned that the names of the various IDs in this RR 
> > will become confusing for devs. We have:
> > 1) `OfferOperationID`, with a field name of `operation_id`
> > 2) `bytes uuid`, with a field name of `uuid` (this is the status update 
> > ID)
> > 3) `bytes uuid`, with a field name of `operation_uuid` (this is our 
> > internal ID for the operation)
> > 
> > What would you think about naming the fields `operation_id`, 
> > `operation_update_uuid`, and `operation_internal_uuid`, respectively? 
> > (could also just do `update_uuid` and `internal_uuid` for the latter two)
> > 
> > Since the fields are in different messages, I'm not sure how bad this 
> > would really be, just a thought. Let me know what you think.

In `Offer.Operation`, the field is `id`, which reads well because the context 
is clear. In `OfferOperationStatus`, it's `operation_id`, which also makes 
sense to me because we might have different `id` in that message.
In `OfferOperation`, it's `operation_uuid`. The reason for `operation_` prefix 
is because there are `framework_id` in the message. Adding a prefix makes the 
context more clear (similar to above). So each offer operation has a `id` 
(specified by the framework) and a `uuid` which is generated by the master.

I agree with you that `uuid` in `OfferOperationStatus` is a bit confusing. How 
about renaming it to `status_uuid`?


- Jie


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


On Oct. 27, 2017, 12:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> ---
> 
> (Updated Oct. 27, 2017, 12:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf definitions related to offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63001/diff/10/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Chun-Hung Hsiao]

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




src/messages/messages.proto
Lines 649 (patched)


s/UPDATE_OPERATION_STATUS/UPDATE_OFFER_OPERATION_STATUS/


- Chun-Hung Hsiao


On Oct. 27, 2017, 12:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> ---
> 
> (Updated Oct. 27, 2017, 12:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf definitions related to offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63001/diff/10/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62903: Added a call to update total resources and pending operations.

[Chun-Hung Hsiao]

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

(Updated Oct. 27, 2017, 8:51 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Now a resource provider first `SUBSCRIBE` to the resource provider
manager without resources to get its ID, then use the ID to prepare the
checkpoints for recovery and persistent work directory, and then
update its total resources and pending operations through
`UPDATE_STATE`.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/resource_provider_manager_tests.cpp 
ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/62903/diff/8/

Changes: https://reviews.apache.org/r/62903/diff/7-8/


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 63372: WIP: Added documentation for the built-in memory profiling.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [63372, 63371, 63370, 63369, 63368, 63367, 63366, 63365]

Failed command: python support/apply-reviews.py -n -r 63366

Error:
2017-10-27 19:49:23 URL:https://reviews.apache.org/r/63366/diff/raw/ 
[10064/10064] -> "63366.patch" [1]
error: missing binary patch data for '3rdparty/jemalloc-5.0.1.tar.bz2'
error: binary patch does not apply to '3rdparty/jemalloc-5.0.1.tar.bz2'
error: 3rdparty/jemalloc-5.0.1.tar.bz2: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19916/console

- Mesos Reviewbot


On Oct. 27, 2017, 7:04 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63372/
> ---
> 
> (Updated Oct. 27, 2017, 7:04 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added documentation for the built-in memory profiling.
> 
> 
> Diffs
> -
> 
>   docs/memory-profiling.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63372/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 63371: WIP: Added unit tests for new MemoryProfiler class.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

WIP: Added unit tests for new MemoryProfiler class.


Diffs
-

  3rdparty/libprocess/src/tests/memory_profiler_tests.cpp PRE-CREATION 


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


Testing
---


Thanks,

Benno Evers

Review Request 63372: WIP: Added documentation for the built-in memory profiling.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

WIP: Added documentation for the built-in memory profiling.


Diffs
-

  docs/memory-profiling.md PRE-CREATION 


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


Testing
---


Thanks,

Benno Evers

Review Request 63366: Added jemalloc release tarball and build rules.

[Benno Evers]

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

Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Repository: mesos


Description
---

This allows building and linking against the bundled
version of jemalloc when the --enable-jemalloc
configuration flag is specified.


Diffs
-

  3rdparty/Makefile.am 687f52d9299bf9c6425c2ba7c35c050ff45a33cd 
  3rdparty/jemalloc-5.0.1.tar.bz2 PRE-CREATION 
  3rdparty/versions.am cbab26d3e303180c3f210f0c5f45613a5c8c96ba 
  configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 


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


Testing
---


Thanks,

Benno Evers

Review Request 63367: Added an overload for strings::endsWith().

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

This saves an unnecessary memory allocation when
testing if a string ends with a string literal.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
067a7923c02342bccd9be1136a981fd6b0e0e9b4 
  3rdparty/stout/tests/strings_tests.cpp 
395540aad88c76a66a43a54edfe9ca1a2d46d3b4 


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


Testing
---


Thanks,

Benno Evers

Review Request 63365: Re-ordered some configuration options alphabetically.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

As part of general preparatory cleanup, some configuration options were 
re-ordered from seemingly random to alphabetical order, and single quotes were 
removed where it could be easily done as they break the code highlighting in 
many popular editors.


Diffs
-

  configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
  src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 


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


Testing
---


Thanks,

Benno Evers

Review Request 63369: Added new '/state' and '/statistics' endpoints to the MemoryProfiler.

[Benno Evers]

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

Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Repository: mesos


Description
---

These endpoints provide useful information for both developers
and end users.


Diffs
-

  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 


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


Testing
---


Thanks,

Benno Evers

Review Request 63370: Added new --memory_profiling flag to agent and master binaries.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

This flag allows explicit disabling of the memory profiler
endpoint in the master and agent binaries.


Diffs
-

  src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
  src/master/flags.cpp 18f405b7e9cc6bd0011284b631974eee52dd2bf3 
  src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
  src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
  src/slave/flags.cpp 789b45b8d84fc01733a885754204fe2fdd6d6807 
  src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 


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


Testing
---


Thanks,

Benno Evers

Review Request 63368: Added MemoryProfiler class to Libprocess.

[Benno Evers]

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

Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Repository: mesos


Description
---

This class exposes profiling functionality of jemalloc memory allocator
when it is detected to be the memory allocator of the current process.

In particular, it gives developers an easy method to collect and
access heap profiles which report which pieces of code were
responsible for allocating memory.


Diffs
-

  3rdparty/libprocess/Makefile.am 03a0ca87f31744c716c99e05aa07242fed480675 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 
dc3375ce62556322eb2bc60ade61f313ade123b8 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62502: Added events to publish and unpublish resources.

[Chun-Hung Hsiao]

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




include/mesos/resource_provider/resource_provider.proto
Lines 87 (patched)


We used to use verbs in `Call` of all other APIs (scheduler, executor, 
etc). The past perfect tense does not look like a "call". How about 
`AcknowledgePublish`?


- Chun-Hung Hsiao


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> ---
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
> https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 62997: Added checkpoint and recover capability for layers in provisioner.

[Zhitao Li]

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

(Updated Oct. 27, 2017, 6:03 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fix populating `info->layers` after provision


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


Repository: mesos


Description
---

Added checkpoint and recover capability for layers in provisioner.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
689acfcbbb07f071b6195472118a7a7520a44abd 
  src/slave/containerizer/mesos/provisioner/paths.hpp 
466f5edab40732b0d8da4252a71fde9c2956e8e9 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
268dbeb4b18374ef53bc73254bf20ce6830e384f 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 


Diff: https://reviews.apache.org/r/62997/diff/3/

Changes: https://reviews.apache.org/r/62997/diff/2-3/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 59687: Added tests for recovering ContainerConfig.

[Zhitao Li]

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

(Updated Oct. 27, 2017, 6:02 p.m.)


Review request for mesos, Gilbert Song and Jason Lai.


Changes
---

Fix format lint.


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


Repository: mesos


Description
---

Added tests for recovering ContainerConfig.


Diffs (updated)
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 


Diff: https://reviews.apache.org/r/59687/diff/3/

Changes: https://reviews.apache.org/r/59687/diff/2-3/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Zhitao Li]

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

(Updated Oct. 27, 2017, 6:01 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Reuse `createTask`.


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


Repository: mesos


Description
---

These tests were using a `TaskInfo` field which is missing required
protobuf fields. After the previous patch begins to checkpoint
`ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
thus caused these tests to fail.

This patch backfills these fields to make these tests pass.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
e61a85df6ec5308ccd2832e66df803b0ad7b53ee 


Diff: https://reviews.apache.org/r/55335/diff/7/

Changes: https://reviews.apache.org/r/55335/diff/6-7/


Testing
---

GTEST_FILTER="MesosContainerizer*" make check


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Oct. 27, 2017, 6 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/13/

Changes: https://reviews.apache.org/r/55334/diff/12-13/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Oct. 27, 2017, 5:57 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Review comments.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/12/

Changes: https://reviews.apache.org/r/55334/diff/11-12/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Eric Chung]

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


Ship it!




Ship It!

- Eric Chung


On Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Oct. 26, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/3/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63094: Added resource version uuid for offer operations.

[Jie Yu]

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

(Updated Oct. 27, 2017, 5:06 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Changes
---

Addressed comments.


Repository: mesos


Description
---

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and changes it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp 
ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63094/diff/4/

Changes: https://reviews.apache.org/r/63094/diff/3-4/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63094: Added resource version uuid for offer operations.

[Jie Yu]

> On Oct. 26, 2017, 11:08 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 712 (patched)
> > 
> >
> > Do we need multiple ResourceVersionUUIDs here? Can the master just 
> > include the UUID for the relevant resource provider/agent?

Yeah, good point.


- Jie


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


On Oct. 19, 2017, 9:35 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> ---
> 
> (Updated Oct. 19, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp 
> ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63358: Added the stub for OfferOperationStatusUpdate handler in the master.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62903.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62903`

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

Relevant logs:

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

```
error: patch failed: include/mesos/resource_provider/resource_provider.proto:70
error: include/mesos/resource_provider/resource_provider.proto: patch does not 
apply
error: patch failed: 
include/mesos/v1/resource_provider/resource_provider.proto:70
error: include/mesos/v1/resource_provider/resource_provider.proto: patch does 
not apply
error: patch failed: src/resource_provider/manager.cpp:155
error: src/resource_provider/manager.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 27, 2017, 10:29 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63358/
> ---
> 
> (Updated Oct. 27, 2017, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the stub for OfferOperationStatusUpdate handler in the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
>   src/master/master.cpp 4b76648b4887724e166ff95e002ee26c8bfef006 
> 
> 
> Diff: https://reviews.apache.org/r/63358/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63314: Add support for Ubuntu 16.04 in docker build.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63314 was successfully built and tested.

Reviews applied: `['63314']`

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

- Mesos Reviewbot Windows


On Oct. 26, 2017, 8:08 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63314/
> ---
> 
> (Updated Oct. 26, 2017, 8:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for Ubuntu 16.04 in docker build.
> 
> 
> Diffs
> -
> 
>   support/docker-build.sh 50e693ca71d499f71f2935923f4691feef560b12 
> 
> 
> Diff: https://reviews.apache.org/r/63314/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62938]

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 Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Oct. 26, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/3/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63358: Added the stub for OfferOperationStatusUpdate handler in the master.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [63358, 63356, 63312, 63105, 61946, 61810, 63104, 63094, 
62903, 63001]

Failed command: python support/apply-reviews.py -n -r 62903

Error:
2017-10-27 13:10:31 URL:https://reviews.apache.org/r/62903/diff/raw/ 
[16204/16204] -> "62903.patch" [1]
error: patch failed: include/mesos/resource_provider/resource_provider.proto:70
error: include/mesos/resource_provider/resource_provider.proto: patch does not 
apply
error: patch failed: 
include/mesos/v1/resource_provider/resource_provider.proto:70
error: include/mesos/v1/resource_provider/resource_provider.proto: patch does 
not apply
error: patch failed: src/resource_provider/manager.cpp:155
error: src/resource_provider/manager.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19913/console

- Mesos Reviewbot


On Oct. 27, 2017, 10:29 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63358/
> ---
> 
> (Updated Oct. 27, 2017, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the stub for OfferOperationStatusUpdate handler in the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
>   src/master/master.cpp 4b76648b4887724e166ff95e002ee26c8bfef006 
> 
> 
> Diff: https://reviews.apache.org/r/63358/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Jie Yu]

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

(Updated Oct. 27, 2017, 12:58 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
Greg Mann, Jan Schlicht, and Vinod Kone.


Changes
---

Addressed Greg's comments.


Repository: mesos


Description
---

Updated protobuf definitions related to offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63001/diff/10/

Changes: https://reviews.apache.org/r/63001/diff/9-10/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63355: Added validation for disk related new operations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63001, 62903, 63094, 63104, 63254, 63255, 63256, 63257, 63355]

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 Oct. 27, 2017, 10:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63355/
> ---
> 
> (Updated Oct. 27, 2017, 10:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> (This is based on https://reviews.apache.org/r/61946)
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp f4925752f20ae8ca4de1d9b4a3d5ffc394db9585 
>   src/master/validation.cpp 42f5742386b59a983f7caaf3400c82b7ef4f8bbb 
>   src/tests/master_validation_tests.cpp 
> 7da1be5233444aded64263d4a763fe2967459042 
> 
> 
> Diff: https://reviews.apache.org/r/63355/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63001: Updated protobuf definitions related to offer operations.

[Jie Yu]

> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > 
> >
> > I think that this list should include pending operations, but should 
> > _not_ include terminal operations with unacknowledged updates.
> > 
> > When the master receives an update, it needs to decide whether or not 
> > it should update the allocator. If the operation was pending when 
> > `UpdateSlaveMessage` was sent, then the master should update the allocator. 
> > If the operation was not pending but its update had not been acknowledged, 
> > then the result of that operation was already reflected in the last 
> > `UpdateSlaveMessage` from the agent, and the master should not update the 
> > allocator.
> > 
> > If the agent includes only pending operations in this field, then the 
> > master will be able to make this distinction.
> 
> Jie Yu wrote:
> The reason we need to send terminal but unacked operations is for 
> reconciliation. Master always keeps a cache about all the known non-completed 
> operations (either pending or terminal but unacked). This information can be 
> used to answer reconciliation requests.
> 
> The master can distinguish between a pending operation vs. a terminal but 
> unacked operation by looking at latest state of the operation. The master 
> update the allocator if the latest state if terminal, irrespective whether 
> it's acked or not.
> 
> That does bring up an issue. Consider the following two cases:
> 
> 1) Master knows about an operation that the agent does not know about. 
> This can happen if the operation gets lost on its way to the agent. This will 
> trigger an agent re-registration. The master in this case will need to 
> generate a reconcile message (similar to that in `reconcileKnownSlave`) so 
> that the agent or LRP can reply with a terminal status update. This will 
> guarantee that the resources allocated for this operation are properly 
> recovered to allocator.
> 
> This case is also possible if there is a speculation failure on old 
> operations and result in an `UpdateSlaveMessage` being sent to the master. 
> The operation that master knows about (but agent does not) will eventually 
> reach agent or LRP, and will be rejected by the LRP or the agent. The master 
> will properly recovered the resources to the allocator when a failed update 
> is received.
> 
> 2) Agent knows about an operation that the master does not know about. 
> For instance, This is a new master that just fails over, and an LRP 
> re-registers with the agent. In that case, the master needs to update the 
> 'allocated' (i.e., 'used') resources for those new operations. This is 
> similar to what we do when slave registers. However, the tricky part is that 
> the allocator does not have an interface currently allowing us to add more 
> allocation to a given agent. We likely need to add that (or make it part of 
> `Allocator::updateSlave` interface).
> 
> Greg Mann wrote:
> OK, I see why we need to include unacked updates as well. Thanks Jie!
> 
> Greg Mann wrote:
> Hey Jie, I have another question related to this issue so I re-opened it 
> for discussion.
> 
> Previously you wrote "The master update the allocator if the latest state 
> if terminal, irrespective whether it's acked or not."
> This implies that subsequent retries of the update will _not_ have the 
> `latest_state` set; otherwise, the master would not be able to distinguish 
> between the first terminal status update and subsequent retries. Was it your 
> intention to suggest that behavior? Looking at the `Slave::forward` helper 
> that we use when intitializing the status update manager, it looks like we 
> _always_ set `latest_state` for task status updates: 
> https://github.com/apache/mesos/blob/f26ffcee0a359a644968feca1ec91243401f589a/src/slave/slave.cpp#L4867-L4870
> 
> I suppose we could set the latest state the first time an operation 
> update is sent to master, and then _not_ set it in subsequent retries. Then 
> the value of `.has_latest_state()` would be a direct indication of whether or 
> not an update is a retry.
> 
> Greg Mann wrote:
> Ah, I see that `latest_status` is optional, so I suspect this was indeed 
> your intention. Just want to confirm :)

Greg, the allocator will be updated only if the state is transitioning from non 
terminal to terminal:
https://github.com/apache/mesos/blob/master/src/master/master.cpp#L9122

No need to strip latest_state when sending the update.


- Jie


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


On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
> 
> 

Review Request 63358: Added the stub for OfferOperationStatusUpdate handler in the master.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

Added the stub for OfferOperationStatusUpdate handler in the master.


Diffs
-

  src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
  src/master/master.cpp 4b76648b4887724e166ff95e002ee26c8bfef006 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63356: Implemented handling of storage related offer operations in master.

[Jie Yu]

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

(Updated Oct. 27, 2017, 10:28 a.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

While the resource provider manager is responsible to apply offer
operations by sending events to the respective resource providers,
the master takes care of accepting these operations. Hence, for local
resource providers the master sends an `ApplyResourceOperationMessage`
to the agent where the resource provider is running on. The agent
then relays the operation contained in the message to the resource
provider manager.

(This is based on https://reviews.apache.org/r/61947)


Diffs
-

  src/common/protobuf_utils.hpp c43ab75b5492320dfe19a7c723a72ac52b8ab722 
  src/common/protobuf_utils.cpp fd4858a64dfc136dd03cb1eef4c97d0f8d43bdae 
  src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
  src/master/master.cpp 4b76648b4887724e166ff95e002ee26c8bfef006 


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


Testing (updated)
---

make check


Thanks,

Jie Yu

Review Request 63356: Implemented handling of storage related offer operations in master.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

While the resource provider manager is responsible to apply offer
operations by sending events to the respective resource providers,
the master takes care of accepting these operations. Hence, for local
resource providers the master sends an `ApplyResourceOperationMessage`
to the agent where the resource provider is running on. The agent
then relays the operation contained in the message to the resource
provider manager.

(This is based on https://reviews.apache.org/r/61947)


Diffs
-

  src/common/protobuf_utils.hpp c43ab75b5492320dfe19a7c723a72ac52b8ab722 
  src/common/protobuf_utils.cpp fd4858a64dfc136dd03cb1eef4c97d0f8d43bdae 
  src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
  src/master/master.cpp 4b76648b4887724e166ff95e002ee26c8bfef006 


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


Testing
---


Thanks,

Jie Yu

Re: Review Request 63312: Updated Resources::apply for new operations.

[Jie Yu]

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

(Updated Oct. 27, 2017, 10:27 a.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.

This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.


Diffs (updated)
-

  include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3 
  include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d 
  src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 
  src/examples/persistent_volume_framework.cpp 
1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf 
  src/master/allocator/mesos/hierarchical.cpp 
5b6efe5faa3c3b10f1f714f582a155b368f8ccaf 
  src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca 


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

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 63355: Added validation for disk related new operations.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan 
Schlicht.


Repository: mesos


Description
---

(This is based on https://reviews.apache.org/r/61946)


Diffs
-

  src/master/validation.hpp f4925752f20ae8ca4de1d9b4a3d5ffc394db9585 
  src/master/validation.cpp 42f5742386b59a983f7caaf3400c82b7ef4f8bbb 
  src/tests/master_validation_tests.cpp 
7da1be5233444aded64263d4a763fe2967459042 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63353: Don't clear the executor ID of non-command executors on re-registration.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63353]

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 Oct. 27, 2017, 7:48 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63353/
> ---
> 
> (Updated Oct. 27, 2017, 7:48 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-8135
> https://issues.apache.org/jira/browse/MESOS-8135
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the agent would sometimes clear the executor ID of
> non-command executors before sending the `ReregisterSlaveMessage`
> message.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d5eb7ba68c5308338236879e7cb1e970a01e48e6 
> 
> 
> Diff: https://reviews.apache.org/r/63353/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the new test fails on GNU/Linux without the rest of the patch, 
> but passes with it.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 63094: Added resource version uuid for offer operations.

[Jie Yu]

> On Oct. 20, 2017, 12:44 p.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 663 (patched)
> > 
> >
> > Even though currently operation on agent resources cannot exhibit 
> > operation speculation failures, I would still prefer would we include some 
> > version uuid for the agent resources here as well.
> > 
> > AFAICT this could happen either by adjusting `ResourceVersionUUID` so 
> > that `resource_provider_id` becomes `optional` or by introducing an 
> > explicit proto3 `map` here, or alternatively, by the agent maintaining its 
> > own uuid which changes whenever a RP uuid changes and using that uuid 
> > instead of a vector here (we discussed this offline).

I'll go with making `resource_provider_id` optional. proto3 map does not 
support a message type key.


- Jie


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


On Oct. 19, 2017, 9:35 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> ---
> 
> (Updated Oct. 19, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp 
> ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 63353: Don't clear the executor ID of non-command executors on re-registration.

[Gaston Kleiman]

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

Review request for mesos, Benjamin Mahler and Greg Mann.


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


Repository: mesos


Description
---

Previously the agent would sometimes clear the executor ID of
non-command executors before sending the `ReregisterSlaveMessage`
message.


Diffs
-

  src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
  src/tests/master_slave_reconciliation_tests.cpp 
d5eb7ba68c5308338236879e7cb1e970a01e48e6 


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


Testing
---

Verified that the new test fails on GNU/Linux without the rest of the patch, 
but passes with it.


Thanks,

Gaston Kleiman

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62938]

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 Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Oct. 26, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/2/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>