Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-25 Thread Benjamin Bannier 

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

(Updated Sept. 25, 2017, 9:42 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comments from Jan.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-25 Thread Jan Schlicht 

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


Fix it, then Ship it!




Look great! Only found some nits.


src/Makefile.am
Lines 1140 (patched)


Move this below `resource_provider/registry.proto` but before 
`resource_provider/storage/provider.hpp` to follow the sorting rules in 
`Makefile.am`.



src/resource_provider/registrar.cpp
Lines 20 (patched)


Put this before ``.



src/resource_provider/registrar.cpp
Lines 138-149 (patched)


Any reason why these functions are implemented here? I'd rather have them 
move above the constructor of `AdmitResourceProvider` to be consistent and 
follow the order of classes in the header.


- Jan Schlicht


On Sept. 25, 2017, 12:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 25, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
>   src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/11/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-25 Thread Benjamin Bannier 

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

(Updated Sept. 25, 2017, 12:27 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-22 Thread Jie Yu 

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


Fix it, then Ship it!





src/tests/resource_provider_manager_tests.cpp
Lines 380 (patched)


I think the indentation here should be 4. ditto below.


- Jie Yu


On Sept. 22, 2017, 3:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 22, 2017, 3:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
>   src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/10/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-22 Thread Benjamin Bannier 


> On Sept. 22, 2017, 2:17 a.m., Jie Yu wrote:
> > src/resource_provider/registrar.cpp
> > Lines 134 (patched)
> > 
> >
> > Instead of wrapping a lot of `#ifndef` in the agent code, i'd suggest 
> > we use the memory based storage on windows and add a TODO in the code.
> > 
> > We can document that windows agent does not support resource providers. 
> > What do you think?

That's a good idea. It prevents us from having to deal with any of this outside 
of the actual agent resource provider registrar. Also see my comment below.


> On Sept. 22, 2017, 2:17 a.m., Jie Yu wrote:
> > src/resource_provider/registrar.cpp
> > Lines 156 (patched)
> > 
> >
> > See my above comments. You can probably just wrap `storage` below.

I ended up introducing an `Owned storage` so the AgentRegistrarProcess 
can use `storage` polymorphically. The Windows workaround is hidden behind a 
storage factory.


> On Sept. 22, 2017, 2:17 a.m., Jie Yu wrote:
> > src/resource_provider/registrar.cpp
> > Lines 201 (patched)
> > 
> >
> > what if fetch failed? Should we fail the recovery?

I believe by propagating the original `Future` in `recover` instead of managing 
our own `Promise` we will allow propagation of the failure to the caller.


- Benjamin


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


On Sept. 22, 2017, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 22, 2017, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
>   src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/10/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-22 Thread Benjamin Bannier 

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

(Updated Sept. 22, 2017, 5:03 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Used `InMemoryStorage` under Windows.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-21 Thread Jie Yu 

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




src/resource_provider/registrar.cpp
Lines 87 (patched)


indent 2 more space here



src/resource_provider/registrar.cpp
Lines 134 (patched)


Instead of wrapping a lot of `#ifndef` in the agent code, i'd suggest we 
use the memory based storage on windows and add a TODO in the code.

We can document that windows agent does not support resource providers. 
What do you think?



src/resource_provider/registrar.cpp
Lines 156 (patched)


See my above comments. You can probably just wrap `storage` below.



src/resource_provider/registrar.cpp
Lines 200 (patched)


I don't think a Promise is needed here, can you just do the following:
```
if (recovered.isNone()) {
  recovered = state.fetch(NAME).then(
  defer(self(),
[this](...) {
  registry = recovery.get();
  variable = recovery;
  return Nothing();
}));
}

return recovered.get();
```



src/resource_provider/registrar.cpp
Lines 201 (patched)


what if fetch failed? Should we fail the recovery?



src/resource_provider/registrar.cpp
Lines 265 (patched)


Why this temp variable?



src/resource_provider/registrar.cpp
Lines 309 (patched)


return here?


- Jie Yu


On Sept. 21, 2017, 7 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 21, 2017, 7 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
>   src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/9/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-21 Thread Benjamin Bannier 

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

(Updated Sept. 21, 2017, 9 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Provided a workaround for Windows.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


Diff: https://reviews.apache.org/r/61528/diff/9/

Changes: https://reviews.apache.org/r/61528/diff/8-9/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-21 Thread Benjamin Bannier 

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

(Updated Sept. 21, 2017, 2:59 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Changed `recover` interface to return `Nothing`, discussed in 
https://reviews.apache.org/r/62353/.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-20 Thread Benjamin Bannier 

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

(Updated Sept. 20, 2017, 4:24 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing (updated)
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-19 Thread Benjamin Bannier 


> On Sept. 19, 2017, 6:30 a.m., Jie Yu wrote:
> > src/resource_provider/registrar.cpp
> > Lines 190 (patched)
> > 
> >
> > I think we want to tie the lifecycle of resource providers to the 
> > lifecycle of agents so that when the agent is gone, all the LRP associated 
> > with that agent is gone too.
> > 
> > Given that we don't change agent ID upon slave machine reboot, i think 
> > it makes sense to checkpoint the local resource provider information under 
> > `/slaves//resource_provider_registry/`
> > 
> > Let's introduce a helper in `src/slave/paths.hpp|cpp` for the path 
> > helper

I put this state below the meta directory now.


> On Sept. 19, 2017, 6:30 a.m., Jie Yu wrote:
> > src/resource_provider/registry.proto
> > Lines 42 (patched)
> > 
> >
> > hum, any reason we don't use:
> > ```
> > message Registry {
> >   repeated ResourceProvider resource_providers;
> > }
> > ```

I followed the style in `master/registry.proto` here. I dropped the extra 
wrapper for now since we do not need it.


- Benjamin


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


On Sept. 19, 2017, 6:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 19, 2017, 6:53 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
>   src/slave/paths.cpp 08177bcef63b998f691be6893ab35891098a2886 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-19 Thread Benjamin Bannier 

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

(Updated Sept. 19, 2017, 6:53 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
  src/slave/paths.cpp 08177bcef63b998f691be6893ab35891098a2886 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-18 Thread Jie Yu 

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




src/master/registry.proto
Lines 73-75 (patched)


Hum, why we still need this?



src/master/registry.proto
Lines 73-75 (patched)


Do we still need this?



src/resource_provider/registrar.hpp
Lines 83 (patched)


Let's be more consistent. We tend not to use tailing underscore for member 
fields if no needed (we do need if there is a public member accessor with the 
same name). Please fix all that in this patch.



src/resource_provider/registrar.cpp
Lines 128-133 (patched)


See my comments below. Probably don't need this if we tweek the protobuf.



src/resource_provider/registrar.cpp
Lines 190 (patched)


I think we want to tie the lifecycle of resource providers to the lifecycle 
of agents so that when the agent is gone, all the LRP associated with that 
agent is gone too.

Given that we don't change agent ID upon slave machine reboot, i think it 
makes sense to checkpoint the local resource provider information under 
`/slaves//resource_provider_registry/`

Let's introduce a helper in `src/slave/paths.hpp|cpp` for the path helper



src/resource_provider/registrar.cpp
Lines 308 (patched)


Please fix all typos.



src/resource_provider/registrar.cpp
Lines 331 (patched)


Given this is a c++ source file, i'll try to use 'using' clause in the 
begining and avoid namespace prefix if possible.



src/resource_provider/registrar.cpp
Lines 337-338 (patched)


Ditto.



src/resource_provider/registry.proto
Lines 42 (patched)


hum, any reason we don't use:
```
message Registry {
  repeated ResourceProvider resource_providers;
}
```


- Jie Yu


On Sept. 15, 2017, 1:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 15, 2017, 1:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-15 Thread Benjamin Bannier 

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

(Updated Sept. 15, 2017, 3:16 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


Diff: https://reviews.apache.org/r/61528/diff/5/

Changes: https://reviews.apache.org/r/61528/diff/4-5/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-07 Thread Benjamin Bannier 

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

(Updated Sept. 8, 2017, 12:36 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing (updated)
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-09-06 Thread Benjamin Bannier 

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

(Updated Sept. 6, 2017, 2:01 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am a4f2ee008eba263fe52c4e58c305e3fb9ba42f6b 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
83a1340fa16b19e3297a8c0ca413afc312de00ec 


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

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


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

2017-08-18 Thread Benjamin Bannier 

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

(Updated Aug. 18, 2017, 5:46 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Followed Jie's implementation suggestion.


Summary (updated)
-

Implemented a registrar for resource provider manager state.


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


Repository: mesos


Description (updated)
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/Makefile.am 68fff148f3d0c1710305bd9afcba62336d194b55 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
85906ea5e1bb3516ef264de22913ce0a3c9c58c5 


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

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


Testing
---


Thanks,

Benjamin Bannier