Re: Review Request 32140: Enabled 'Resources' to handle 'Resource::ReservationInfo'.

2015-04-23 Thread Alexander Rukletsov
On April 15, 2015, 5:36 p.m., Alexander Rukletsov wrote: src/common/resources.cpp, lines 93-105 https://reviews.apache.org/r/32140/diff/5/?file=920081#file920081line93 Can we replace it by ``` if !addable(left, right) { return false

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-29 Thread Alexander Rukletsov
/#comment132515 This looks a bit weird. Is it what `clang-format` proposes? - Alexander Rukletsov On April 28, 2015, 10:43 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-29 Thread Alexander Rukletsov
On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: src/master/master.cpp, lines 2458-2459 https://reviews.apache.org/r/32150/diff/5/?file=920953#file920953line2458 These two fields are optional, `principal` doesn't have a default. Do we need to check it? Can a framework

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-30 Thread Alexander Rukletsov
On April 29, 2015, 9:54 a.m., Alexander Rukletsov wrote: src/master/master.cpp, lines 2486-2488 https://reviews.apache.org/r/32150/diff/5-8/?file=920953#file920953line2486 This looks a bit weird. Is it what `clang-format` proposes? Michael Park wrote: It is indeed. Do you

Re: Review Request 32140: Enabled 'Resources' to handle 'Resource::ReservationInfo'.

2015-04-28 Thread Alexander Rukletsov
On April 15, 2015, 5:36 p.m., Alexander Rukletsov wrote: src/common/resources.cpp, lines 455-464 https://reviews.apache.org/r/32140/diff/5/?file=920081#file920081line455 Agree with Jie, I spent some time trying to understand what's going on here and what the implications

Re: Review Request 33718: Extended documentation on Mesos hooks.

2015-05-07 Thread Alexander Rukletsov
a8b471541cdfa584eeb89fbe96643f93c712cfd4 Diff: https://reviews.apache.org/r/33718/diff/ Testing --- none: docs update. @Adam: as a native speaker, do you mind checking the language? Thanks, Alexander Rukletsov

Re: Review Request 33514: Wired up --allocator flag in master.

2015-05-08 Thread Alexander Rukletsov
=ExternalAllocatorModule ./bin/mesos-slave.sh --master=master-ip --work_dir=s/work ``` Slave successfully registers, its resources are offered to a simple task: ``` ./src/mesos-execute --command=sleep 1 --master=master-ip --name=alloc-test ``` Thanks, Alexander Rukletsov

Re: Review Request 33718: Extended documentation on Mesos hooks.

2015-05-08 Thread Alexander Rukletsov
that. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33718/#review82908 --- On May 8, 2015, 11:19 p.m., Alexander Rukletsov wrote

Re: Review Request 33513: Added a modules-aware factory for allocators.

2015-05-08 Thread Alexander Rukletsov
Diff: https://reviews.apache.org/r/33513/diff/ Testing --- make check (Mac OS 10.9.5, CentOS 7.0) Thanks, Alexander Rukletsov

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-13 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/#review83609 --- Ship it! Ship It! - Alexander Rukletsov On May 12, 2015, 6:44

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-13 Thread Alexander Rukletsov
On April 24, 2015, 8:18 a.m., Alexander Rukletsov wrote: src/common/resources.cpp, lines 442-445 https://reviews.apache.org/r/32398/diff/2/?file=921003#file921003line442 Let's use this function where we already check for this condition, like in `master/validation.cpp

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-19 Thread Alexander Rukletsov
/#comment135528 You may consider using `.at()` or `.get()` instead. They both have `const` accessors. - Alexander Rukletsov On May 19, 2015, 8:42 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-19 Thread Alexander Rukletsov
On May 18, 2015, 1:18 p.m., Till Toenshoff wrote: src/master/http.cpp, line 312 https://reviews.apache.org/r/30612/diff/15/?file=947349#file947349line312 `const`? Alexander Rojas wrote: Cannot be const because the operator `[]` provides no const overload (common problem on

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-23 Thread Alexander Rukletsov
On June 22, 2015, 10:20 a.m., Alexander Rukletsov wrote: Before making a thorough review, let's discuss one high level question. Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-20 Thread Alexander Rukletsov
shouldn't worry too much. I am fine with sending a status update. Alexander Rukletsov wrote: I wonder, what are the cases when the task launch request may arrive before `CheckpointResourcesMessage`? If my understanding is correct, we do not have delivery guarantee

Re: Review Request 35687: Added capabilities to state.json

2015-06-20 Thread Alexander Rukletsov
/#comment141227 Let's make it `foreachvalue` for consistency. - Alexander Rukletsov On June 20, 2015, 7:14 a.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35687

Re: Review Request 35583: Updated slave to exit on authentication refusal.

2015-06-20 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35583/#review88641 --- Ship it! Ship It! - Alexander Rukletsov On June 17, 2015, 10:42

Re: Review Request 35686: Change the if statement to a CHECK at the end of _runTask.

2015-06-20 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35686/#review88635 --- Ship it! Ship It! - Alexander Rukletsov On June 20, 2015, 4:42

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Alexander Rukletsov
/master/http.cpp (line 553) https://reviews.apache.org/r/35702/#comment141344 s/recinding/rescinding src/master/http.cpp (line 560) https://reviews.apache.org/r/35702/#comment141345 s/Recinding/rescinding/ - Alexander Rukletsov On June 22, 2015, 5:29 a.m., Michael Park wrote

Re: Review Request 35433: Send StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-18 Thread Alexander Rukletsov
. - Alexander Rukletsov On June 18, 2015, 5:01 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-19 Thread Alexander Rukletsov
/slave/slave.cpp (line 1446) https://reviews.apache.org/r/35433/#comment141096 Ditto. - Alexander Rukletsov On June 19, 2015, 12:42 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 35647: Fix logic for calling erase inside loop.

2015-06-19 Thread Alexander Rukletsov
()`? - Alexander Rukletsov On June 19, 2015, 12:24 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35647

Re: Review Request 35571: Adding ability to decode JSON from ZK

2015-06-19 Thread Alexander Rukletsov
` here? - Alexander Rukletsov On June 18, 2015, 3:31 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-19 Thread Alexander Rukletsov
, 12:42 a.m.) Review request for mesos, Alexander Rukletsov, Benjamin Hindman, and Jie Yu. Repository: mesos Description --- No bug was observed (yet), but realized I forgot about this in the dynamic reservations patches. Diffs - src/slave/slave.cpp

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

2015-06-19 Thread Alexander Rukletsov
On June 19, 2015, 9:26 a.m., Alexander Rukletsov wrote: src/slave/slave.cpp, line 1280 https://reviews.apache.org/r/35638/diff/1/?file=987726#file987726line1280 Though currently we guarantee `executorInfo` lifetime is not shorter than `executorId`'s, I think we agreed to disallow

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

2015-06-19 Thread Alexander Rukletsov
://reviews.apache.org/r/35638/#comment141092 Ditto - Alexander Rukletsov On June 19, 2015, 12:43 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638

Re: Review Request 35694: Added helper constructors to hashmap.

2015-06-20 Thread Alexander Rukletsov
iterator = map.begin(), auto endIterator = map.end(); iterator != endIterator; ++iterator) { ``` 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp (line 32) https://reviews.apache.org/r/35694/#comment141249 Let's expand the test for move c-tor as well. - Alexander

Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.

2015-06-22 Thread Alexander Rukletsov
) https://reviews.apache.org/r/35702/#comment141371 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve`? - Alexander Rukletsov On June 22, 2015, 5:29 a.m., Michael Park wrote

Re: Review Request 35694: Added helper constructors to hashmap.

2015-06-22 Thread Alexander Rukletsov
On June 20, 2015, 7:35 p.m., Alexander Rukletsov wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp, line 52 https://reviews.apache.org/r/35694/diff/3/?file=988938#file988938line52 Let's avoid re-creating iterator: ``` for (auto iterator

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

2015-06-22 Thread Alexander Rukletsov
it? Was the test flaky? - Alexander Rukletsov On June 20, 2015, 8:51 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35703

Re: Review Request 34545: Updated the allocator related docs.

2015-05-29 Thread Alexander Rukletsov
/configuration.md 5a41477d81eceff98eef4c2ba096448fc408ff3a docs/modules.md 835c195745421938a5dc86c3c7d5777e02d660b7 Diff: https://reviews.apache.org/r/34545/diff/ Testing --- Rendered the docs in MacDown and github. Thanks, Alexander Rukletsov

Re: Review Request 34545: Updated the allocator related docs.

2015-05-29 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34545/#review85723 --- On May 29, 2015, 3:39 p.m., Alexander Rukletsov wrote

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-02 Thread Alexander Rukletsov
? - Alexander Rukletsov On June 28, 2015, 8:37 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35983

Re: Review Request 35927: Added TaskStatus::Reason to Termination Message.

2015-06-28 Thread Alexander Rukletsov
checking for reason? src/tests/docker_containerizer_tests.cpp https://reviews.apache.org/r/35927/#comment142331 ditto - Alexander Rukletsov On June 26, 2015, 4:26 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail

Re: Review Request 35816: Fixed the incorrect CHECK_EQ in updateAllocation.

2015-06-28 Thread Alexander Rukletsov
://reviews.apache.org/r/35816/#comment142321 Why do you pause the clock here? - Alexander Rukletsov On June 24, 2015, 3:35 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 33159: Pump updateFramework through Allocator from Master.

2015-05-22 Thread Alexander Rukletsov
throwing a comment about that? - Alexander Rukletsov On April 20, 2015, 6:46 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159

Re: Review Request 34545: Updated the allocator related docs.

2015-05-26 Thread Alexander Rukletsov
/configuration.md 5a41477d81eceff98eef4c2ba096448fc408ff3a docs/modules.md 835c195745421938a5dc86c3c7d5777e02d660b7 Diff: https://reviews.apache.org/r/34545/diff/ Testing --- Rendered the docs in MacDown and github. Thanks, Alexander Rukletsov

Re: Review Request 34545: Updated the allocator related docs.

2015-05-21 Thread Alexander Rukletsov
54c4e31ed6dfed3c23d492c19a301ce119a0519b docs/modules.md 835c195745421938a5dc86c3c7d5777e02d660b7 Diff: https://reviews.apache.org/r/34545/diff/ Testing --- Rendered the docs in MacDown and github. Thanks, Alexander Rukletsov

Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.

2015-08-21 Thread Alexander Rukletsov
a timeout, which is naturally expressed by `unavailability.start`. Given we don't need duration in this case, the name can be misleading for users. - Alexander Rukletsov On Aug. 12, 2015, 10:07 p.m., Joseph Wu wrote

Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.

2015-08-23 Thread Alexander Rukletsov
On Aug. 21, 2015, 6:35 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, lines 917-920 https://reviews.apache.org/r/36321/diff/9/?file=1038857#file1038857line917 I think the name `Unavailability` is too specific to maintenance, how about something more generic, like

Re: Review Request 35927: Added TaskStatus::Reason to Termination Message.

2015-06-29 Thread Alexander Rukletsov
On June 28, 2015, 8:57 a.m., Alexander Rukletsov wrote: include/mesos/containerizer/containerizer.proto, lines 87-89 https://reviews.apache.org/r/35927/diff/1/?file=993029#file993029line87 Why do we need a deprecation cycle for this? AFAIK, it's part of the containerizer API only

Review Request 36916: Doxygenified a comment in the allocator.proto.

2015-07-29 Thread Alexander Rukletsov
Description --- Doxygenified a comment in the allocator.proto. Diffs - include/mesos/master/allocator.proto c32b88ed0252bf71e738ee64d64ffa828e97b2a3 Diff: https://reviews.apache.org/r/36916/diff/ Testing --- none. Thanks, Alexander Rukletsov

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 515-516 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515 It looks like we actually have the role, but it's buried in resources. Do you envision having resources collection

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-07-29 Thread Alexander Rukletsov
/#comment147835 s/Method/method ? - Alexander Rukletsov On July 29, 2015, 3:05 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913

Review Request 36883: Fixed a typo in oversubscription doc.

2015-07-28 Thread Alexander Rukletsov
--- See summary. Diffs - docs/oversubscription.md 4ff1f654d526664f15aa8bbc722f4188dbb12897 Diff: https://reviews.apache.org/r/36883/diff/ Testing --- None. Thanks, Alexander Rukletsov

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, line 507 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line507 The code until this line is basically request validation and authorization. Though it's not how we do it now, do you

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 1325-1332 https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325 Why do we need to recover resources for unreserve? Michael Park wrote: If reserved resources are offered, we

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote: src/master/master.cpp, line 749 https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote: src/master/http.cpp, line 1291 https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1291 As in https://reviews.apache.org/r/35702/, I suggest we extract validation into a separate function. Michael Park

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On July 2, 2015, 3:39 p.m., Alexander Rukletsov wrote: A high level question: do operators have the possibility to get a list of all dynamic reservations? I think about a situation, when a framework made some reservations and then quit, an operator wants to clean up those

Re: Review Request 35947: Added a new API call 'updateAvailable' to the allocator.

2015-07-28 Thread Alexander Rukletsov
On July 8, 2015, 5:59 p.m., Alexander Rukletsov wrote: include/mesos/master/allocator.hpp, lines 133-135 https://reviews.apache.org/r/35947/diff/1/?file=993649#file993649line133 And we introduce a libprocess dependency into `Allocator` interface. I think it's a prominent step

Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.

2015-07-29 Thread Alexander Rukletsov
/#comment147904 Shouldn't it come after network shutdown? https://reviews.apache.org/r/36920/ - Alexander Rukletsov On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply

Review Request 36920: Fixed TearDown order in ZK test.

2015-07-29 Thread Alexander Rukletsov
) Thanks, Alexander Rukletsov

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-08-04 Thread Alexander Rukletsov
/36913/#comment148533 s/and / - Alexander Rukletsov On Aug. 4, 2015, 9:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-08-04 Thread Alexander Rukletsov
On Aug. 3, 2015, 2:21 p.m., Alexander Rukletsov wrote: src/master/quota_handler.hpp, lines 26-27 https://reviews.apache.org/r/36913/diff/5/?file=1027610#file1027610line26 I think we should wrap it into `master` namespace as well. `quota_handler.cpp` still lacks the `master

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-08-04 Thread Alexander Rukletsov
/36913/#comment148528 I think we don't need this instance any more. - Alexander Rukletsov On Aug. 3, 2015, 6:09 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-08-04 Thread Alexander Rukletsov
/36913/#comment148537 Let's add something like: Operates inside the Master actor. - Alexander Rukletsov On Aug. 4, 2015, 9:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Alexander Rukletsov
On July 28, 2015, 8:43 a.m., Alexander Rukletsov wrote: Also, I think we need a doppelgänger for the method in the `streaming` namespace. Joerg Schad wrote: Are you sure? For delete we are not streaming any data... I think we do it implicitly anyway (see `StreamingResponseDecoder

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov
On July 16, 2015, 2:54 p.m., Alexander Rukletsov wrote: src/master/http.cpp, line 447 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line447 Not directly related to endpoints, but to dynamic reservations in general. Do you think it makes sense to bookkeep

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov
On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 1325-1332 https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325 Why do we need to recover resources for unreserve? Michael Park wrote: If reserved resources are offered, we

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov
On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote: src/master/master.cpp, line 749 https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 515-516 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515 It looks like we actually have the role, but it's buried in resources. Do you envision having resources collection

Re: Review Request 36847: Added HTTP Delete Method.

2015-07-28 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review93307 --- Ship it! Ship It! - Alexander Rukletsov On July 28, 2015, 4:26

Re: Review Request 36663: Added ip_address field to MasterInfo

2015-07-29 Thread Alexander Rukletsov
/36663/#comment147803 `ip` and `port` are required, while `address` is optional. Is it intentional / doesn't it introduce a pitfall? - Alexander Rukletsov On July 25, 2015, 12:36 a.m., Marco Massenzio wrote

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-08-03 Thread Alexander Rukletsov
/36913/#comment148310 A minor nit: let's sort them as they appear in the RFC: GET POST PUT DELETE - Alexander Rukletsov On July 29, 2015, 8:48 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 36663: Added ip_address field to MasterInfo

2015-07-30 Thread Alexander Rukletsov
On July 29, 2015, 1:01 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, line 399 https://reviews.apache.org/r/36663/diff/5/?file=1021578#file1021578line399 `ip` and `port` are required, while `address` is optional. Is it intentional / doesn't it introduce a pitfall

Re: Review Request 36908: Added QuotaInfo Protobuf.

2015-07-29 Thread Alexander Rukletsov
Can we do something like `master/%.pb.cc ../include/mesos/master/%.pb.h: $(ALLOCATOR_PROTO) $(QUOTA_PROTO)` for clarity? src/Makefile.am (lines 482 - 484) https://reviews.apache.org/r/36908/#comment147823 Hard tabs here as well, please. - Alexander Rukletsov On July 29, 2015, 11:54 a.m

Review Request 36911: Removed unnecessary using directive.

2015-07-29 Thread Alexander Rukletsov
: mesos Description --- Removed unnecessary using directive. Diffs - src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a Diff: https://reviews.apache.org/r/36911/diff/ Testing --- make check (clang on Mac OS 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-14 Thread Alexander Rukletsov
On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote: src/master/master.cpp, line 749 https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-16 Thread Alexander Rukletsov
/#comment145598 Not directly related to endpoints, but to dynamic reservations in general. Do you think it makes sense to bookkeep dynamic reservation or have an aggregating method in `mesos::internal::master::Role`? - Alexander Rukletsov On June 28, 2015, 8:36 a.m., Michael Park wrote

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

2015-07-16 Thread Alexander Rukletsov
/#comment145599 As in https://reviews.apache.org/r/35702/, I suggest we extract validation into a separate function. src/master/http.cpp (lines 1325 - 1332) https://reviews.apache.org/r/35983/#comment145600 Why do we need to recover resources for unreserve? - Alexander Rukletsov

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-13 Thread Alexander Rukletsov
the comment for `applyOfferOperation()` as well. - Alexander Rukletsov On June 28, 2015, 8:36 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702

Re: Review Request 35687: Added capabilities to state.json

2015-07-13 Thread Alexander Rukletsov
`FrameworkInfo::Capability::Type_Name()` IMO. - Alexander Rukletsov On July 12, 2015, 6:25 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35687

Re: Review Request 35687: Added capabilities to state.json

2015-07-13 Thread Alexander Rukletsov
On June 20, 2015, 10:50 a.m., Alexander Rukletsov wrote: src/master/http.cpp, line 124 https://reviews.apache.org/r/35687/diff/1/?file=988871#file988871line124 Let's make it `foreachvalue` for consistency. Aditi Dixit wrote: Hi, I'm sorry, but I'm unable to understand

Re: Review Request 39285: Added Quota Request Validation.

2015-10-22 Thread Alexander Rukletsov
tps://reviews.apache.org/r/39285/#comment161683> Should this function be public, or protected is good enough? - Alexander Rukletsov On Oct. 22, 2015, 4:38 a.m., Joerg Schad wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 39590: Made license-headers doxygen-compatible.

2015-10-24 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39590/#review103903 --- What about `.proto` files? - Alexander Rukletsov On Oct. 24

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-23 Thread Alexander Rukletsov
://reviews.apache.org/r/39401/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

2015-10-23 Thread Alexander Rukletsov
://reviews.apache.org/r/39450/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

2015-10-23 Thread Alexander Rukletsov
/hierarchical.cpp f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 Diff: https://reviews.apache.org/r/39400/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-10-23 Thread Alexander Rukletsov
://reviews.apache.org/r/38218/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39399: Quota: Refactored hierarchical allocator in preparation for quota.

2015-10-23 Thread Alexander Rukletsov
/hierarchical.cpp f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 Diff: https://reviews.apache.org/r/39399/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39317: Quota: Moved QuotaInfo protobuf into a separate package.

2015-10-23 Thread Alexander Rukletsov
include/mesos/master/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 src/CMakeLists.txt e6169a0e3ad34dd0e4c3430a6532bd48c4bd04fd src/Makefile.am 98cbafc134ec388a176d50172912fbfdf9f5bfa3 Diff: https://reviews.apache.org/r/39317/diff/ Testing --- make check Thanks, Alexander

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-10-22 Thread Alexander Rukletsov
518 --- On Oct. 22, 2015, 1:03 a.m., Joerg Schad wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36913/ > --------

Re: Review Request 39507: Check message.empty() in Slave::shutdown before log it

2015-10-21 Thread Alexander Rukletsov
g/r/39507/#comment161483> Capitalize? src/slave/slave.cpp (line 658) <https://reviews.apache.org/r/39507/#comment161484> Ditto - Alexander Rukletsov On Oct. 21, 2015, 2:39 a.m., Guangya Liu wrote: > > --- > Thi

Re: Review Request 39285: Added Quota Request Validation.

2015-10-22 Thread Alexander Rukletsov
. src/master/quota_handler.cpp (line 159) <https://reviews.apache.org/r/39285/#comment161615> We do not support update currently, so the message is misleading. How about "Quota cannot be set for a role that already has quota"?

Re: Review Request 39285: Added Quota Request Validation.

2015-10-22 Thread Alexander Rukletsov
tps://reviews.apache.org/r/39285/#comment161662> We don't need this comment any more, right? - Alexander Rukletsov On Oct. 22, 2015, 4:38 a.m., Joerg Schad wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov
sources every time when we allocate > some resources to a framework :-) I see your point. I'll change the behaviour. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39401/#review103

Re: Review Request 39400: Quota: Implemented quota API.

2015-10-29 Thread Alexander Rukletsov
, nothing prevents a role from having spaces in its name. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39400/#review104265 ---

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 Diff: https://reviews.apache.org/r/39401/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov
> quota is satisfied. After all the quota’ed role has been traversed, if > > there are still some role’s quotas are not satisfied, then lay aside > > resources (should be the resources filtered or suppressed) for them. In > > this way, before laying aside resources, we have

Re: Review Request 39400: Quota: Implemented quota API.

2015-10-27 Thread Alexander Rukletsov
pplies. But you're right, it's misleading. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39400/#review103984 -------

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

2015-10-27 Thread Alexander Rukletsov
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 Diff: https://reviews.apache.org/r/39450/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 Diff: https://reviews.apache.org/r/39401/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

2015-10-27 Thread Alexander Rukletsov
cfd937ba306273c24fb5337dfeb1a15e1545169b src/master/allocator/mesos/hierarchical.cpp f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 Diff: https://reviews.apache.org/r/39400/diff/ Testing --- make check (Mac OS X 10.10.4) Thanks, Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov
> quota is satisfied. After all the quota’ed role has been traversed, if > > there are still some role’s quotas are not satisfied, then lay aside > > resources (should be the resources filtered or suppressed) for them. In > > this way, before laying aside resources, we have

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov
mail. To reply, visit: https://reviews.apache.org/r/39401/#review104013 ------- On Oct. 27, 2015, 7:27 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically gener

Re: Review Request 39590: Made license-headers doxygen-compatible.

2015-10-26 Thread Alexander Rukletsov
> On Oct. 24, 2015, 7:23 p.m., Alexander Rukletsov wrote: > > What about `.proto` files? > > Benjamin Bannier wrote: > Good point, since they contain C++ embedded they should follow a similar > style. I pushed an updated RR. > > Most files follow t

Re: Review Request 39400: Quota: Implemented quota API.

2015-10-26 Thread Alexander Rukletsov
Also when shall we need to add "" for one parameter in log message? I think we are inconsistent. AFAIK, @neilc is doing some cleanup around. - Alexander --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39401/#review104010 --- On Oct. 23, 2015, 4:38 p.m., Alexander Rukletsov wrote: > > ---

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov
s not what we want. So I guess it > > should be: > > ``` > > if (allocatable(resources)) { > > // Lay aside the resources. > > laidAsideResources[slaveId] = resources; > > slaves[slaveId].allocated += resources; > >

  1   2   3   4   5   6   7   8   9   10   >