Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96366 --- include/mesos/mesos.proto (line 914) https://reviews.apache.org/r/36321/#comment151698 Got it. include/mesos/mesos.proto (line 939) https://reviews.apache.org/r/36321/#comment151696 Thanks Joe! Got it, but I think that it is better that we can add some notes here to be more clear - Guangya Liu On Aug. 25, 2015, 3:24 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:24 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37175: Maintenance Primitives: Added updateUnavailability to master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37175/#review96370 --- After the TimeSpec change: src/master/master.cpp (lines 4139 - 4143) https://reviews.apache.org/r/37175/#comment151700 Should be possible to do something like: ``` Duration start = Seconds(unavailability.get().start().seconds()) + Nanoseconds(unavailability.get().start().nanoseconds()); LOG(INFO) ... start.secs(); ``` - Joseph Wu On Aug. 24, 2015, 7:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37175/ --- (Updated Aug. 24, 2015, 7:12 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb Diff: https://reviews.apache.org/r/37175/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96372 --- Ship it! src/slave/containerizer/provisioners/appc/paths.hpp (line 52) https://reviews.apache.org/r/37722/#comment151710 s/appc/APPC/ src/slave/containerizer/provisioners/appc/paths.hpp (line 53) https://reviews.apache.org/r/37722/#comment151712 How about the following layout: `APPC/containers/container_id` IN that way, it's more easy to add any metadata that is not associated with any container. src/slave/containerizer/provisioners/appc/paths.hpp (line 54) https://reviews.apache.org/r/37722/#comment151716 Similarly, can you use the following layout: `container_id/backends/backend/...` src/slave/containerizer/provisioners/appc/paths.hpp (line 59) https://reviews.apache.org/r/37722/#comment151717 s/docker/DOCKER/ src/slave/paths.cpp (line 457) https://reviews.apache.org/r/37722/#comment151702 You can use stringify(imageType) directly. You might need to add a operator for Image::Type - Jie Yu On Aug. 24, 2015, 6:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 6:26 p.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37177: Maintenance Primitives: Added inverse offers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37177/#review96375 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37177/ --- (Updated Aug. 25, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/hierarchical_allocator_tests.cpp 9748ca0b3fee25dcec51c64d8ba84dbd4aaf src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc Diff: https://reviews.apache.org/r/37177/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37189: Added std::hash template specializations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37189/#review96355 --- Ship it! Ship It! - Michael Park On Aug. 25, 2015, 1:16 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37189/ --- (Updated Aug. 25, 2015, 1:16 p.m.) Review request for mesos, Alexander Rojas and Michael Park. Bugs: MESOS-3217 https://issues.apache.org/jira/browse/MESOS-3217 Repository: mesos Description --- Added std::hash template specializations. Diffs - include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/linux/cgroups.hpp 91ccfd046256d29504e93003de83c04f048c9b78 src/master/metrics.hpp c51887ee1e072a063d629a735a3c6a2212e616ef src/messages/log.hpp 94e38c189703ff22c412b86429e052e942e75e75 src/module/manager.hpp cab67a8b1ee7164bb3cb2a8b24e1a6e05b40fd19 src/module/manager.cpp 862b71fa9d1e303e98de3767d69c9805e7b26dcc src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 src/tests/fetcher_tests.cpp 81e70368f7a6164e9649964881aa5a688ef222e8 src/tests/module.hpp 850c2002882be52e25f191f17785698deb5513c1 Diff: https://reviews.apache.org/r/37189/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36404: Added support for peek() to process::io
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/#review96356 --- 3rdparty/libprocess/src/io.cpp (lines 36 - 37) https://reviews.apache.org/r/36404/#comment151674 +2 not +4 please. - Benjamin Hindman On Aug. 6, 2015, 3:26 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/ --- (Updated Aug. 6, 2015, 3:26 a.m.) Review request for mesos, Joris Van Remoortere and Joseph Wu. Bugs: MESOS-2964 https://issues.apache.org/jira/browse/MESOS-2964 Repository: mesos Description --- JIRA: https://issues.apache.org/jira/browse/MESOS-2964 Diffs - 3rdparty/libprocess/include/process/io.hpp 975923f40f82357f31b89428f24d01df6a8ac9fc 3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 3rdparty/libprocess/src/tests/io_tests.cpp c642bab9e2845668767ad237985cb9ce1109 Diff: https://reviews.apache.org/r/36404/diff/ Testing --- - Added a test case for process::io::peek - make check Thanks, Artem Harutyunyan
Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/ --- (Updated Aug. 25, 2015, 10:03 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Adjust some spacing in the tests (correcting the style). Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.start Registry operation = maintenance::StartMaintenance Sets the list of machines as Deactivated. Diffs (updated) - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37358/diff/ Testing --- `make check` New Tests: RegistrarTest.StartMaintenance Schedules some machines. Deactivates some. MasterMaintenanceTest.StartMaintenance Tests some invalid lists. Schedules some machines. Tests some valid and invalid lists. Thanks, Joseph Wu
Re: Review Request 37362: Maintenance Primitives: Adds an endpoint for transitioning agents back into Normal mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37362/ --- (Updated Aug. 25, 2015, 10:03 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Adjust some spacing in the tests (correcting the style). Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.stop Registry operation = maintenance::StopMaintenance Sets the list of machines back to Normal mode. Removes those machines from the maintenance schedule. Diffs (updated) - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37362/diff/ Testing --- `make check` New Tests: RegistrarTest.StopMaintenance Schedules some machines. Deactivates some. Reactivates some. MasterMaintenanceTest.ReactivateMachines Tests some invalid lists. Schedules some machines. Deactivates some. Tests some valid and invalid lists. Checks that the schedule is modified. Thanks, Joseph Wu
Re: Review Request 37325: Maintenance Primitives: Adds an endpoint for scheduling agents for maintenance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37325/ --- (Updated Aug. 25, 2015, 10:03 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Adjust some spacing in the tests (correcting the style). Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.schedule Registry operation = maintenance::UpdateSchedule Replaces the schedule with the given one. Also sets all scheduled machines into Draining mode. Other changes: Added a note about the strict flag. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 src/tests/maintenance.hpp PRE-CREATION src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37325/diff/ Testing --- `make check` New Tests: RegistrarTest.UpdateMaintenanceSchedule Schedules 3 machines, 1 at a time. Rearranges schedules. Checks that machines are put into Draining mode. Removes machines. MasterMaintenanceTest.UpdateSchedule Hits the new endpoint with some valid and invalid schedules. Only tests a subset of invalid schedules (requires other endpoints to fully test). Thanks, Joseph Wu
Re: Review Request 37114: MESOS-3187, support docker host command line option
On Aug. 25, 2015, 5:05 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 102 https://reviews.apache.org/r/37114/diff/4/?file=1047856#file1047856line102 Also does this mean if a user put in tcp:/// we're just padding unix:///? I think it's safe to assume we should always expect a absolute path to the socket, and I'm not sure what other prefix are we allowing here by allowing a custom prefix to be passed in. The default value of the flag does not come with absolute path and changing would break the logic of slaves containerization as we discussed ealier (flags.hpp). The code checks for prefix presence before adding it. I can make this check for other values - tcp:// and fd://. If no prefix is found, I would assume no value at command line is provided and the default value is flags.hpp is being passed, thus add unix:// prefix. Let me know if you are okay with solution? - Vaibhav --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96360 --- On Aug. 21, 2015, 8:54 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 21, 2015, 8:54 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37172/#review96365 --- After the TimeSpec change: src/tests/master_maintenance_tests.cpp (line 258) https://reviews.apache.org/r/37172/#comment151695 Might need a `static_castint64_t` here. src/tests/master_maintenance_tests.cpp (line 277) https://reviews.apache.org/r/37172/#comment151694 Should be changed to an int comparison. - Joseph Wu On Aug. 24, 2015, 7:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37172/ --- (Updated Aug. 24, 2015, 7:12 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37172/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37188: Added std::hash template specializations.
On Aug. 25, 2015, 12:36 p.m., Michael Park wrote: 3rdparty/libprocess/src/tests/http_tests.cpp, lines 677-695 https://reviews.apache.org/r/37188/diff/6/?file=1037577#file1037577line677 I think this might be the right solution for the scope of the changes we're trying to make, but I don't think we'll want to keep this kind of tests. My preference would be to change `URL` to operate on `map` rather than `hashmap`. Jan Schlicht wrote: Agreed! Could you file a JIRA ticket describing the issue and a few simple solutions for it? 1. Make `URL` operate on `map` rather than `hashmap` 2. Only update the `stringify` function to construct a temporary `map` from the `hashmap` in order to print them out in a deterministic order) 3. ... anything else you can think of Thanks! - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37188/#review96329 --- On Aug. 25, 2015, 1:15 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37188/ --- (Updated Aug. 25, 2015, 1:15 p.m.) Review request for mesos, Alexander Rojas and Michael Park. Bugs: MESOS-3217 https://issues.apache.org/jira/browse/MESOS-3217 Repository: mesos Description --- Added std::hash template specializations. Diffs - 3rdparty/libprocess/include/process/address.hpp df78f8e525c40b87e734e16979d3315f89e12594 3rdparty/libprocess/include/process/pid.hpp 8d3735c7d5b8f74a7a0ebb8cafe7c7ebee68f5f0 3rdparty/libprocess/src/pid.cpp f5528aea04a17ce868a0f67b94defdefea18e234 3rdparty/libprocess/src/tests/http_tests.cpp c22f9d35b5dc959ce9816d3358ebff92b853bf52 Diff: https://reviews.apache.org/r/37188/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 37362: Maintenance Primitives: Adds an endpoint for transitioning agents back into Normal mode.
On Aug. 25, 2015, 2:11 a.m., Guangya Liu wrote: src/master/maintenance.cpp, line 153 https://reviews.apache.org/r/37362/diff/7/?file=1048631#file1048631line153 Not quite under what does the paramter strict means here? Seems it was not used. See my comment: https://reviews.apache.org/r/37358/#comment151628 On Aug. 25, 2015, 2:11 a.m., Guangya Liu wrote: src/master/maintenance.cpp, line 157 https://reviews.apache.org/r/37362/diff/7/?file=1048631#file1048631line157 I see that changed will be set to true when one machine removed from mantain list, what does changed means here? See my comment: https://reviews.apache.org/r/37358/#comment151627 - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37362/#review96313 --- On Aug. 24, 2015, 12:08 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37362/ --- (Updated Aug. 24, 2015, 12:08 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.stop Registry operation = maintenance::StopMaintenance Sets the list of machines back to Normal mode. Removes those machines from the maintenance schedule. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37362/diff/ Testing --- `make check` New Tests: RegistrarTest.StopMaintenance Schedules some machines. Deactivates some. Reactivates some. MasterMaintenanceTest.ReactivateMachines Tests some invalid lists. Schedules some machines. Deactivates some. Tests some valid and invalid lists. Checks that the schedule is modified. Thanks, Joseph Wu
Re: Review Request 36404: Added support for peek() to process::io
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/#review96342 --- 3rdparty/libprocess/include/process/io.hpp (line 145) https://reviews.apache.org/r/36404/#comment151656 I would like us to document the parameters please, specifically it's not obvious the difference between 'size' and 'limit'. IIUC we'll return right away even if we haven't read all of 'size'? Same for the overload below. 3rdparty/libprocess/src/io.cpp (lines 69 - 70) https://reviews.apache.org/r/36404/#comment151672 Same line please. 3rdparty/libprocess/src/io.cpp (line 71) https://reviews.apache.org/r/36404/#comment151673 How about adding a comment that if 'fd' is not a socket we'll do the right thing because 'recv' will fail with ENOTSOCK and we'll propagate that out. 3rdparty/libprocess/src/io.cpp (lines 274 - 286) https://reviews.apache.org/r/36404/#comment151671 This is the old style, in the new style we just dupliate the file descriptor so that if someone closes the file descriptor passed to us we don't either read from a closed file descriptor or WORSE read from a newly opened file descriptor that we shouldn't be reading from. See 'io::read(int fd)' for an example. Note that the other ones need to get changed as well but I've done that for the Mesos on Windows work (in the my github/benh/mesos mesos-on-windows branch, which was necessary to do there because we can't support os::isNonblock on Windows). 3rdparty/libprocess/src/io.cpp (line 400) https://reviews.apache.org/r/36404/#comment151670 The other internal::_* functions were originally created to (A) deal with the fact we didn't have C++11 and (B) because they were recursive. We don't need that for 'peek' in this case, and in the other cases we can ultimately remove the need for the internal function and just use a recursive lambda. So, how about killing internal::_peek? 3rdparty/libprocess/src/io.cpp (line 407) https://reviews.apache.org/r/36404/#comment151662 I just checked and we've used 'length' throughout this file for this variable name, let's keep it consistent for now and do the same here please. 3rdparty/libprocess/src/io.cpp (line 577) https://reviews.apache.org/r/36404/#comment151658 The declaration uses the variable named 'limit' but here it's 'size' which sort of implies a different semantics? Either way, we should use the same name please. 3rdparty/libprocess/src/io.cpp (line 582) https://reviews.apache.org/r/36404/#comment151659 This indentation should be +2 not +4. 3rdparty/libprocess/src/io.cpp (line 588) https://reviews.apache.org/r/36404/#comment151661 This appears to be an unused variable? Or am I missing something? Which will also render the TODO(bmahler) above useless. - Benjamin Hindman On Aug. 6, 2015, 3:26 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/ --- (Updated Aug. 6, 2015, 3:26 a.m.) Review request for mesos, Joris Van Remoortere and Joseph Wu. Bugs: MESOS-2964 https://issues.apache.org/jira/browse/MESOS-2964 Repository: mesos Description --- JIRA: https://issues.apache.org/jira/browse/MESOS-2964 Diffs - 3rdparty/libprocess/include/process/io.hpp 975923f40f82357f31b89428f24d01df6a8ac9fc 3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 3rdparty/libprocess/src/tests/io_tests.cpp c642bab9e2845668767ad237985cb9ce1109 Diff: https://reviews.apache.org/r/36404/diff/ Testing --- - Added a test case for process::io::peek - make check Thanks, Artem Harutyunyan
Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/#review96368 --- src/master/maintenance.cpp (line 121) https://reviews.apache.org/r/37358/#comment151699 Got it - Guangya Liu On Aug. 25, 2015, 5:03 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/ --- (Updated Aug. 25, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.start Registry operation = maintenance::StartMaintenance Sets the list of machines as Deactivated. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37358/diff/ Testing --- `make check` New Tests: RegistrarTest.StartMaintenance Schedules some machines. Deactivates some. MasterMaintenanceTest.StartMaintenance Tests some invalid lists. Schedules some machines. Tests some valid and invalid lists. Thanks, Joseph Wu
Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/#review96369 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 5:03 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/ --- (Updated Aug. 25, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.start Registry operation = maintenance::StartMaintenance Sets the list of machines as Deactivated. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37358/diff/ Testing --- `make check` New Tests: RegistrarTest.StartMaintenance Schedules some machines. Deactivates some. MasterMaintenanceTest.StartMaintenance Tests some invalid lists. Schedules some machines. Tests some valid and invalid lists. Thanks, Joseph Wu
Review Request 37747: Introduced bind-mount based provisioner Backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/ --- Review request for mesos, Lily Chen, Jie Yu, Timothy Chen, and Vinod Kone. Bugs: MESOS-3190 https://issues.apache.org/jira/browse/MESOS-3190 Repository: mesos Description --- Introduced bind-mount based provisioner Backend. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/backend.cpp 6190ce3eeff6ea22142c9eaa5a771ae1b767740c src/slave/containerizer/provisioners/backends/bind.hpp PRE-CREATION src/slave/containerizer/provisioners/backends/bind.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 src/tests/containerizer/provisioner_backend_tests.cpp PRE-CREATION src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 Diff: https://reviews.apache.org/r/37747/diff/ Testing --- sudo make check. Added one test. Thanks, Jiang Yan Xu
Re: Review Request 37582: Maintenance Primitives: Add test for the hierarchical DRF allocator sending inverse offers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37582/ --- (Updated Aug. 25, 2015, 10:39 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Update test to use TimeSpec in Unavailability. Bugs: MESOS-3042 https://issues.apache.org/jira/browse/MESOS-3042 Repository: mesos Description --- Adds a test for the previous review in this chain. Diffs (updated) - src/tests/hierarchical_allocator_tests.cpp 9748ca0b3fee25dcec51c64d8ba84dbd4aaf Diff: https://reviews.apache.org/r/37582/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37281: Maintenance Primitives: Added Unavailability to Offer in V1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37281/#review96381 --- I think that this can be abandoned, as https://reviews.apache.org/r/36321/diff/11#0 is already covering this part and also the time is using TimeSpec to be more accurate. - Guangya Liu On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37281/ --- (Updated Aug. 25, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37281/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37282: Maintenance Primitives: Added InverseOffer to V1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37282/#review96382 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37282/ --- (Updated Aug. 25, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 src/internal/devolve.hpp 6e4306df78b9b8d2054e6550209341fd7b0972d6 src/internal/devolve.cpp 0a069e51053b572a8d5dc95380732119504dd0c9 src/internal/evolve.hpp 13e9f52da98567038ec717f394f79e526a1521e9 src/internal/evolve.cpp 11ce9e77490e93f781ceebc33063d13953a11765 src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c Diff: https://reviews.apache.org/r/37282/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.
On Aug. 24, 2015, 11:04 p.m., Guangya Liu wrote: include/mesos/mesos.proto, line 111 https://reviews.apache.org/r/36571/diff/14/?file=1048532#file1048532line111 Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine. In this case, it doesn't matter *why* the user puts 2+ agents on a single machine. It matters that the user *can* put 2+ agents on a single machine. On Aug. 24, 2015, 11:04 p.m., Guangya Liu wrote: include/mesos/maintenance/maintenance.proto, line 19 https://reviews.apache.org/r/36571/diff/14/?file=1048531#file1048531line19 I think that you will handle v1 api in other patches? That is correct. Specifically, for the maintenance protobufs, we will add it to the V1 API when it is necessary to do so. - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/#review96301 --- On Aug. 24, 2015, 11:33 a.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/ --- (Updated Aug. 24, 2015, 11:33 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3066 https://issues.apache.org/jira/browse/MESOS-3066 Repository: mesos Description --- * MachineInfo - Describes a single box that holds one or more agents. * MachineInfos - A list of boxes. * maintenance::Window - A set of machines and a planned downtime period. * maintenance::Schedule - A set of maintenance windows. * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated. * Registry::MaintenanceStatus - Holds the maintenance mode of a machine. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION include/mesos/maintenance/maintenance.proto PRE-CREATION include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 Diff: https://reviews.apache.org/r/36571/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
On Aug. 24, 2015, 5:57 p.m., Guangya Liu wrote: include/mesos/mesos.proto, line 939 https://reviews.apache.org/r/36321/diff/10/?file=1048529#file1048529line939 Can you please elaborate more for why in maintaince purpose, this field is always empty? Why cannot an operator set the resources that s/he want to inverse? Two reasons: * Maintenance primitives is currently an MVP. So an operator can only schedule a whole machine. We have plans for finer granularity, but that's not implemented yet. * InverseOffers are not maintenance-specific. We mention maintenance in the comment because this functionality is built-in. On Aug. 24, 2015, 5:57 p.m., Guangya Liu wrote: include/mesos/mesos.proto, line 914 https://reviews.apache.org/r/36321/diff/10/?file=1048529#file1048529line914 s/(i.e. which slave)/(i.e. which framework) I think that the comments for this may need to be updated as here if no framework id is specified, then all framework's resources will be request back. The language here is correct. As with offers, the `required FrameworkID` is required and designates the framework receiving the offer. Since only one framework receives an offer at once, it does not make sense to tell one framework to deallocate resources from another framework. On Aug. 24, 2015, 5:57 p.m., Guangya Liu wrote: include/mesos/mesos.proto, line 125 https://reviews.apache.org/r/36321/diff/10/?file=1048529#file1048529line125 Unavailability is a time interval or period, but from the name of Unavailability, someone may not able to understand it specifies the time interval or period for maintain. What about using name UnavailableInterval or UnavailablePeriod which might be more meaningful? Availability, semantically, already includes a concept of time (i.e. When are you available?). - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96259 --- On Aug. 24, 2015, 11:26 a.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 24, 2015, 11:26 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 8:24 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Change Unavailability to use TimeSpec for time. Copy over to V1 API. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs (updated) - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
On Aug. 25, 2015, 12:27 p.m., Michael Park wrote:
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp, lines 142-172
https://reviews.apache.org/r/37187/diff/5/?file=1037650#file1037650line142
Given that there's no specified iteration order of a multihashmap, I
don't think this is even something to test for.
That is, this test doesn't test anything further than the test above it:
```
ASSERT_EQ(2u, map.get(foo).size());
ASSERT_TRUE(map.contains(foo, 1024));
ASSERT_TRUE(map.contains(foo, 1025));
```
I would say we should just remove it, what do you think?
Jan Schlicht wrote:
IMHO the purpose of this test is to show that the iterator iteratates
over all elements of a multihashmap, not that the iterater provides a
specific order. But given that that this is not too complicated it's probably
safe to remove this test.
Michael Park wrote:
Yeah, let's remove this test.
Hm, how about we actually leave the test but follow the pattern for the
`ForEach` test to ignore the ordering?
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/#review96331
---
On Aug. 25, 2015, 4:02 p.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 25, 2015, 4:02 p.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
On Aug. 25, 2015, 12:27 p.m., Michael Park wrote:
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp, lines 142-172
https://reviews.apache.org/r/37187/diff/5/?file=1037650#file1037650line142
Given that there's no specified iteration order of a multihashmap, I
don't think this is even something to test for.
That is, this test doesn't test anything further than the test above it:
```
ASSERT_EQ(2u, map.get(foo).size());
ASSERT_TRUE(map.contains(foo, 1024));
ASSERT_TRUE(map.contains(foo, 1025));
```
I would say we should just remove it, what do you think?
Jan Schlicht wrote:
IMHO the purpose of this test is to show that the iterator iteratates
over all elements of a multihashmap, not that the iterater provides a
specific order. But given that that this is not too complicated it's probably
safe to remove this test.
Yeah, let's remove this test.
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/#review96331
---
On Aug. 11, 2015, 11:57 a.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 11, 2015, 11:57 a.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Re: Review Request 37325: Maintenance Primitives: Adds an endpoint for scheduling agents for maintenance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37325/ --- (Updated Aug. 25, 2015, 9:10 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Update Unavailability to use integer precision. Changes the two Maintenance.hpp files (one for validation and one as a test helper). Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.schedule Registry operation = maintenance::UpdateSchedule Replaces the schedule with the given one. Also sets all scheduled machines into Draining mode. Other changes: Added a note about the strict flag. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 src/tests/maintenance.hpp PRE-CREATION src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37325/diff/ Testing --- `make check` New Tests: RegistrarTest.UpdateMaintenanceSchedule Schedules 3 machines, 1 at a time. Rearranges schedules. Checks that machines are put into Draining mode. Removes machines. MasterMaintenanceTest.UpdateSchedule Hits the new endpoint with some valid and invalid schedules. Only tests a subset of invalid schedules (requires other endpoints to fully test). Thanks, Joseph Wu
Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/#review96359 --- Ship it! Ship It! - Guangya Liu On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/ --- (Updated Aug. 24, 2015, 6:33 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3066 https://issues.apache.org/jira/browse/MESOS-3066 Repository: mesos Description --- * MachineInfo - Describes a single box that holds one or more agents. * MachineInfos - A list of boxes. * maintenance::Window - A set of machines and a planned downtime period. * maintenance::Schedule - A set of maintenance windows. * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated. * Registry::MaintenanceStatus - Holds the maintenance mode of a machine. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION include/mesos/maintenance/maintenance.proto PRE-CREATION include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 Diff: https://reviews.apache.org/r/36571/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/
---
(Updated Aug. 25, 2015, 10:07 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan,
Joris Van Remoortere, and Vinod Kone.
Changes
---
Adjust some spacing in the tests (correcting the style).
Bugs: MESOS-2067
https://issues.apache.org/jira/browse/MESOS-2067
Repository: mesos
Description
---
Endpoint: /maintenance.status
Returns a JSON object like:
```
{
draining: [
{ hostname: foo, ip: 0.0.0.1 },
{ hostname: bar, ip: 0.0.0.2 },
],
deactivated: [
{ hostname: baz, ip: 0.0.0.3 },
]
}
```
Diffs (updated)
-
src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc
src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
src/tests/master_maintenance_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37364/diff/
Testing
---
`make check`
New Tests:
MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
Extra test case for the /maintenance.schedule endpoint, which requires all
three endpoints to work.
MasterMaintenanceTest.MachineStatus
Schedules, starts, and stops maintenance. Checks machine statuses after
each step.
Thanks,
Joseph Wu
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 25, 2015, 6:02 p.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Changes
---
Removed multimap iterator test.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs (updated)
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/#review96348
---
Ship it!
Ship It!
- Michael Park
On Aug. 25, 2015, 4:02 p.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 25, 2015, 4:02 p.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
On Aug. 25, 2015, 1:36 a.m., Guangya Liu wrote: src/master/maintenance.cpp, line 121 https://reviews.apache.org/r/37358/diff/5/?file=1048624#file1048624line121 What does strict means here? I see it was not used This is explained in the previous review: https://reviews.apache.org/r/37325/diff/11#6 Essentially, this is the `registry_strict` flag, which only applies to operations which affect masters/agents infos in the registry. Hence, we do not use it for maintenance. On Aug. 25, 2015, 1:36 a.m., Guangya Liu wrote: src/master/maintenance.cpp, line 133 https://reviews.apache.org/r/37358/diff/5/?file=1048624#file1048624line133 Not quite understand the logic here, what does changed means? If one machine put into DEACTIVATED mode, then the changed will be translated to true and it will be true for the whole life cycle of this function. This is to satisfy the prototype of an Operation. `Operation::perform` returns a bool indicating if the registry was changed as a result of the operation. - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/#review96309 --- On Aug. 24, 2015, 11:54 a.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/ --- (Updated Aug. 24, 2015, 11:54 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.start Registry operation = maintenance::StartMaintenance Sets the list of machines as Deactivated. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37358/diff/ Testing --- `make check` New Tests: RegistrarTest.StartMaintenance Schedules some machines. Deactivates some. MasterMaintenanceTest.StartMaintenance Tests some invalid lists. Schedules some machines. Tests some valid and invalid lists. Thanks, Joseph Wu
Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- (Updated Aug. 25, 2015, 4:40 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Addressed comments. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs (updated) - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96360 --- src/docker/docker.hpp (line 44) https://reviews.apache.org/r/37114/#comment151677 End comment with period. src/docker/docker.hpp (line 46) https://reviews.apache.org/r/37114/#comment151678 Align this with the last line (one space to the left), also if you look at the next method example of our default values in parameters, follow the same spacing (space between = and ) src/docker/docker.hpp (line 161) https://reviews.apache.org/r/37114/#comment151680 Format spacing correctly, look at other examples in the code. src/docker/docker.cpp (line 99) https://reviews.apache.org/r/37114/#comment151682 Format spacing src/docker/docker.cpp (line 101) https://reviews.apache.org/r/37114/#comment151683 End comment with period. src/docker/docker.cpp (line 102) https://reviews.apache.org/r/37114/#comment151689 Also does this mean if a user put in tcp:/// we're just padding unix:///? I think it's safe to assume we should always expect a absolute path to the socket, and I'm not sure what other prefix are we allowing here by allowing a custom prefix to be passed in. src/docker/docker.cpp (line 105) https://reviews.apache.org/r/37114/#comment151684 two spaces src/docker/docker.cpp (line 107) https://reviews.apache.org/r/37114/#comment151685 Why paranethsis? Also spacing src/docker/docker.cpp (line 663) https://reviews.apache.org/r/37114/#comment151690 Correct formating by moving paranethis aligning with last line src/docker/executor.hpp (line 45) https://reviews.apache.org/r/37114/#comment151691 s/host/socket/g - Timothy Chen On Aug. 21, 2015, 8:54 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 21, 2015, 8:54 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port
Re: Review Request 37189: Added std::hash template specializations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37189/#review96336 --- Patch looks great! Reviews applied: [37187, 37188, 37189] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 1:16 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37189/ --- (Updated Aug. 25, 2015, 1:16 p.m.) Review request for mesos, Alexander Rojas and Michael Park. Bugs: MESOS-3217 https://issues.apache.org/jira/browse/MESOS-3217 Repository: mesos Description --- Added std::hash template specializations. Diffs - include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/linux/cgroups.hpp 91ccfd046256d29504e93003de83c04f048c9b78 src/master/metrics.hpp c51887ee1e072a063d629a735a3c6a2212e616ef src/messages/log.hpp 94e38c189703ff22c412b86429e052e942e75e75 src/module/manager.hpp cab67a8b1ee7164bb3cb2a8b24e1a6e05b40fd19 src/module/manager.cpp 862b71fa9d1e303e98de3767d69c9805e7b26dcc src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 src/tests/fetcher_tests.cpp 81e70368f7a6164e9649964881aa5a688ef222e8 src/tests/module.hpp 850c2002882be52e25f191f17785698deb5513c1 Diff: https://reviews.apache.org/r/37189/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 37784: Remove the redundant check in HierarchicalDRFAlocator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37784/ --- Review request for mesos. Bugs: MESOS-3301 https://issues.apache.org/jira/browse/MESOS-3301 Repository: mesos Description --- Remove the redundant check in HierarchicalDRFAlocator. Diffs - src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e Diff: https://reviews.apache.org/r/37784/diff/ Testing --- Thanks, Yong Qiao Wang
Re: Review Request 37784: Remove the redundant check in HierarchicalDRFAlocator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37784/#review96488 --- Patch looks great! Reviews applied: [37784] All tests passed. - Mesos ReviewBot On Aug. 26, 2015, 2:54 a.m., Yong Qiao Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37784/ --- (Updated Aug. 26, 2015, 2:54 a.m.) Review request for mesos. Bugs: MESOS-3301 https://issues.apache.org/jira/browse/MESOS-3301 Repository: mesos Description --- Remove the redundant check in HierarchicalDRFAlocator. Diffs - src/master/allocator/mesos/hierarchical.hpp 38f8fd2c84314bb3731684d0e9795cb4f50a227e Diff: https://reviews.apache.org/r/37784/diff/ Testing --- Thanks, Yong Qiao Wang
Re: Review Request 37787: Reworked Jenkins build script. Added test images for ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-6.6-gcc-4.8.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37787/#review96501 --- Patch looks great! Reviews applied: [37787] All tests passed. - Mesos ReviewBot On Aug. 26, 2015, 5:43 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37787/ --- (Updated Aug. 26, 2015, 5:43 a.m.) Review request for Benjamin Hindman, Timothy Chen and Vinod Kone. Bugs: MESOS-3313 https://issues.apache.org/jira/browse/MESOS-3313 Repository: mesos Description --- Reworked Jenkins build script. Added test images for ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-6.6-gcc-4.8. Diffs - support/docker/centos-6.6-gcc-4.8/Dockerfile PRE-CREATION support/docker/centos-6.6-gcc-4.8/wandisco-svn.repo PRE-CREATION support/docker/centos-7.1-gcc-4.8/Dockerfile PRE-CREATION support/docker/ubuntu-12.04-gcc-4.8/Dockerfile PRE-CREATION support/docker/ubuntu-14.04-clang-3.6/Dockerfile PRE-CREATION support/docker/ubuntu-14.04-gcc-4.8/Dockerfile PRE-CREATION support/jenkins_build_docker.sh PRE-CREATION Diff: https://reviews.apache.org/r/37787/diff/ Testing --- `jenkins-build-docker.sh` is a reworked version of the original `jenkins-build.sh` that is ran by Jenkins buildbot for building and testing Mesos distributions. Features: * Runs libevent, SSL and ROOT tests. * Easily add OS/compiler Docker images for testing Mesos. * Exclude tests on per-image basis. * Easily reproduce the test image locally. * Three new test images (ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-6.6-gcc-4.8). How to run The following environment variables have to be set for the script to run: * OS - OS name/version. Currently images are available for ubuntu-14.04, ubuntu-12.04, centos-7.1, centos-6.6. * CONFIGURATION - ./configure flags (e.g. '--enable-libevent'). * COMPILER - Compiler name/version. Currently available images include gcc-4.8 (default value) on all platforms, clang-3.6 on ubuntu-14.04. Examples: `OS=ubuntu-14.04 CONFIGURATION='--enable-ssl --enable-libevent' COMPILER=clang-3.6 ./jenkins_build_docker.sh` `OS=centos-7.1 CONFIGURATION='--enable-ssl --enable-libevent' ./jenkins_build_docker.sh` NOTE: Mesos Python module has a known issue on centos-6.6 ( https://issues.apache.org/jira/browse/MESOS-3314 ), so **for now centos-6.6 should not be enabled in Jenkins**. Thanks, Artem Harutyunyan
Review Request 37785: Fix Flaky SlaveTest.HTTPSchedulerSlaveRestart test
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37785/ --- Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-3311 https://issues.apache.org/jira/browse/MESOS-3311 Repository: mesos Description --- I was not able to reproduce this with 300 gtest iterations in a loop on a Ubuntu 14.04 VM with clang + ssl i.e. similar to the ASF setup. The logs though made it pretty evident on what was going on. The slave was sending a retry re-register message to the master, resulting in the master sending back another FrameworkUpdateMessage, the 2nd one used to set the PID from None() to the original pid() making the message go through directly to the scheduler instead of being routed through the master. Log Lines: I0825 22:07:39.085610 27642 slave.cpp:1209] Will retry registration in 6.014445ms if necessary I0825 22:07:39.092914 27640 master.cpp:3773] Re-registering slave 20150825-220736-234885548-51219-27610-S0 at slave(286)@172.17.0.14:51219 (09c6504e3a31) I0825 22:07:39.093181 27630 slave.cpp:1209] Will retry registration in 20.588077ms if necessary some lines and then I0825 22:07:39.094435 27640 master.cpp:3773] Re-registering slave 20150825-220736-234885548-51219-27610-S0 at slave(287)@172.17.0.14:51219 (09c6504e3a31) ... more lines I0825 22:07:39.096372 27635 slave.cpp:2131] Updating framework 20150825-220736-234885548-51219-27610- pid to @0.0.0.0:0 ... more lines I0825 22:07:39.097450 27635 slave.cpp:2131] Updating framework 20150825-220736-234885548-51219-27610- pid to scheduler-6c5ddcdb-9dd1-4b38-b051-5f714d3c1c55@172.17.0.14:51219 ... more lines I0825 22:07:39.098433 27635 slave.cpp:3043] Sending message for framework 20150825-220736-234885548-51219-27610- to scheduler-6c5ddcdb-9dd1-4b38-b051-5f714d3c1c55@172.17.0.14:51219 Paused the clock and then settle/resume invocations to ensure the retry does not happen Diffs - src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37785/diff/ Testing --- make check again with 300 iterations without failure Thanks, Anand Mazumdar
Re: Review Request 37234: Maintenance Primitives: Added URL field to InverseOffer proto.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37234/#review96500 --- Ship it! Ship It! - Guangya Liu On Aug. 26, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37234/ --- (Updated Aug. 26, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf Diff: https://reviews.apache.org/r/37234/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 36404: Added support for peek() to process::io
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/ --- (Updated Aug. 25, 2015, 10:17 p.m.) Review request for Joris Van Remoortere and Joseph Wu. Changes --- Addressed some comments. Bugs: MESOS-2964 https://issues.apache.org/jira/browse/MESOS-2964 Repository: mesos Description --- JIRA: https://issues.apache.org/jira/browse/MESOS-2964 Diffs (updated) - 3rdparty/libprocess/include/process/io.hpp 975923f40f82357f31b89428f24d01df6a8ac9fc 3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 3rdparty/libprocess/src/tests/io_tests.cpp c642bab9e2845668767ad237985cb9ce1109 Diff: https://reviews.apache.org/r/36404/diff/ Testing --- - Added a test case for process::io::peek - make check Thanks, Artem Harutyunyan
Re: Review Request 37785: Fix Flaky SlaveTest.HTTPSchedulerSlaveRestart test
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37785/#review96491 --- Patch looks great! Reviews applied: [37785] All tests passed. - Mesos ReviewBot On Aug. 26, 2015, 3:07 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37785/ --- (Updated Aug. 26, 2015, 3:07 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-3311 https://issues.apache.org/jira/browse/MESOS-3311 Repository: mesos Description --- I was not able to reproduce this with 300 gtest iterations in a loop on a Ubuntu 14.04 VM with clang + ssl i.e. similar to the ASF setup. The logs though made it pretty evident on what was going on. The slave was sending a retry re-register message to the master, resulting in the master sending back another FrameworkUpdateMessage, the 2nd one used to set the PID from None() to the original pid() making the message go through directly to the scheduler instead of being routed through the master. Log Lines: I0825 22:07:39.085610 27642 slave.cpp:1209] Will retry registration in 6.014445ms if necessary I0825 22:07:39.092914 27640 master.cpp:3773] Re-registering slave 20150825-220736-234885548-51219-27610-S0 at slave(286)@172.17.0.14:51219 (09c6504e3a31) I0825 22:07:39.093181 27630 slave.cpp:1209] Will retry registration in 20.588077ms if necessary some lines and then I0825 22:07:39.094435 27640 master.cpp:3773] Re-registering slave 20150825-220736-234885548-51219-27610-S0 at slave(287)@172.17.0.14:51219 (09c6504e3a31) ... more lines I0825 22:07:39.096372 27635 slave.cpp:2131] Updating framework 20150825-220736-234885548-51219-27610- pid to @0.0.0.0:0 ... more lines I0825 22:07:39.097450 27635 slave.cpp:2131] Updating framework 20150825-220736-234885548-51219-27610- pid to scheduler-6c5ddcdb-9dd1-4b38-b051-5f714d3c1c55@172.17.0.14:51219 ... more lines I0825 22:07:39.098433 27635 slave.cpp:3043] Sending message for framework 20150825-220736-234885548-51219-27610- to scheduler-6c5ddcdb-9dd1-4b38-b051-5f714d3c1c55@172.17.0.14:51219 Paused the clock and then settle/resume invocations to ensure the retry does not happen Diffs - src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37785/diff/ Testing --- make check again with 300 iterations without failure Thanks, Anand Mazumdar
Re: Review Request 36404: Added support for peek() to process::io
On Aug. 25, 2015, 9:29 a.m., Benjamin Hindman wrote: 3rdparty/libprocess/src/io.cpp, lines 274-286 https://reviews.apache.org/r/36404/diff/7/?file=1027739#file1027739line274 This is the old style, in the new style we just dupliate the file descriptor so that if someone closes the file descriptor passed to us we don't either read from a closed file descriptor or WORSE read from a newly opened file descriptor that we shouldn't be reading from. See 'io::read(int fd)' for an example. Note that the other ones need to get changed as well but I've done that for the Mesos on Windows work (in the my github/benh/mesos mesos-on-windows branch, which was necessary to do there because we can't support os::isNonblock on Windows). I initially had the dup() here (it's in this same review) but then Joris pointed out that ::recv() with MSG_PEEK set can not surpass single message boundaries. I removed the call to dup() because unlike the case with io::read() we are not looping here and are performing just a single peek() request. I will look into this further and will update accordingly. On Aug. 25, 2015, 9:29 a.m., Benjamin Hindman wrote: 3rdparty/libprocess/src/io.cpp, line 577 https://reviews.apache.org/r/36404/diff/7/?file=1027739#file1027739line577 The declaration uses the variable named 'limit' but here it's 'size' which sort of implies a different semantics? Either way, we should use the same name please. That was an oversight. Fixed. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/#review96342 --- On Aug. 5, 2015, 8:26 p.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36404/ --- (Updated Aug. 5, 2015, 8:26 p.m.) Review request for mesos, Joris Van Remoortere and Joseph Wu. Bugs: MESOS-2964 https://issues.apache.org/jira/browse/MESOS-2964 Repository: mesos Description --- JIRA: https://issues.apache.org/jira/browse/MESOS-2964 Diffs - 3rdparty/libprocess/include/process/io.hpp 975923f40f82357f31b89428f24d01df6a8ac9fc 3rdparty/libprocess/src/io.cpp 4a6e18a17012994d358099ad32d4c282fea3b0b1 3rdparty/libprocess/src/tests/io_tests.cpp c642bab9e2845668767ad237985cb9ce1109 Diff: https://reviews.apache.org/r/36404/diff/ Testing --- - Added a test case for process::io::peek - make check Thanks, Artem Harutyunyan
Re: Review Request 36908: Added QuotaInfo Protobuf.
On Aug. 25, 2015, 5:51 a.m., Guangya Liu wrote: src/Makefile.am, line 182 https://reviews.apache.org/r/36908/diff/4/?file=1032680#file1032680line182 It is better consider the alpha order of those files, we can add quota related files under module related files. Alexander Rukletsov wrote: Could you please elaborate? I'm not sure I follow. Guangya Liu wrote: I mean the file order in Makefile.am should follow alpha order as other part in this file. My propose is that you only need to re-order the file list here as following: module/module.pb.cc \ ../include/mesos/module/module.pb.h \ master/quota.pb.cc \ ../include/mesos/master/quota.pb.h\ Just adjust the order should works. I see. We also include folder into lexicographic sorting, therefore `module master` and hence all `master/**` go before `module/**`. Does it make sense? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review96299 --- On Aug. 5, 2015, 2:03 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated Aug. 5, 2015, 2:03 p.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 37325: Maintenance Primitives: Adds an endpoint for scheduling agents for maintenance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37325/#review96308 --- Ship it! Ship It! - Guangya Liu On Aug. 24, 2015, 6:48 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37325/ --- (Updated Aug. 24, 2015, 6:48 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.schedule Registry operation = maintenance::UpdateSchedule Replaces the schedule with the given one. Also sets all scheduled machines into Draining mode. Other changes: Added a note about the strict flag. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 src/tests/maintenance.hpp PRE-CREATION src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37325/diff/ Testing --- `make check` New Tests: RegistrarTest.UpdateMaintenanceSchedule Schedules 3 machines, 1 at a time. Rearranges schedules. Checks that machines are put into Draining mode. Removes machines. MasterMaintenanceTest.UpdateSchedule Hits the new endpoint with some valid and invalid schedules. Only tests a subset of invalid schedules (requires other endpoints to fully test). Thanks, Joseph Wu
Re: Review Request 37358: Maintenance Primitives: Adds an endpoint for transitioning agents into the Deactivated maintenance mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/#review96309 --- src/master/maintenance.cpp (line 121) https://reviews.apache.org/r/37358/#comment151628 What does strict means here? I see it was not used src/master/maintenance.cpp (line 133) https://reviews.apache.org/r/37358/#comment151627 Not quite understand the logic here, what does changed means? If one machine put into DEACTIVATED mode, then the changed will be translated to true and it will be true for the whole life cycle of this function. - Guangya Liu On Aug. 24, 2015, 6:54 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37358/ --- (Updated Aug. 24, 2015, 6:54 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.start Registry operation = maintenance::StartMaintenance Sets the list of machines as Deactivated. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37358/diff/ Testing --- `make check` New Tests: RegistrarTest.StartMaintenance Schedules some machines. Deactivates some. MasterMaintenanceTest.StartMaintenance Tests some invalid lists. Schedules some machines. Tests some valid and invalid lists. Thanks, Joseph Wu
Re: Review Request 37314: Maintenance Primitives: Populate master's maintenance-related local state upon recovery from registry.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37314/#review96302 --- Ship it! Ship It! - Guangya Liu On Aug. 24, 2015, 6:43 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37314/ --- (Updated Aug. 24, 2015, 6:43 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3045 https://issues.apache.org/jira/browse/MESOS-3045 Repository: mesos Description --- Note: Tests will be added with the related HTTP endpoints and registrar operations, later in this review chain. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf Diff: https://reviews.apache.org/r/37314/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 25, 2015, 11:41 a.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Changes --- Comments. NNFR. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs (updated) - include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 src/tests/paths_tests.cpp 9b7179691bb8c146e5d5cca7437ec64a456a38ad Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37497: Added Docker provisioner paths which handles path manipulation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37497/ --- (Updated Aug. 25, 2015, 6:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added Docker provisioner paths which handles path manipulation. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37497/diff/ Testing --- sudo make check ./bin/mesos-tests.sh --gtest_filter=*DockerProvisioner* --gtest_repeat=20 --gtest_shuffle=1 Thanks, Lily Chen
Re: Review Request 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37280/#review96386 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37280/ --- (Updated Aug. 25, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb Diff: https://reviews.apache.org/r/37280/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 25, 2015, 6:47 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37198: Add Docker image provisioner and copy backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/ --- (Updated Aug. 25, 2015, 6:48 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2850 https://issues.apache.org/jira/browse/MESOS-2850 Repository: mesos Description --- Add Docker image provisioner and copy backend. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/isolators/filesystem/linux.cpp f36424e94c380870cfde49d55af397fa3dc4a612 src/slave/containerizer/provisioner.hpp 541dd4e0b2f0c92a45c00cab6132a2be69654838 src/slave/containerizer/provisioner.cpp efc7e6996ff6663bebaf61989a7e040bd2ad7a5e src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 src/tests/containerizer/provisioner.hpp c4ba46794fe5d7875fda11155367f521c34ea339 Diff: https://reviews.apache.org/r/37198/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- (Updated Aug. 25, 2015, 6:48 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs (updated) - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37582: Maintenance Primitives: Add test for the hierarchical DRF allocator sending inverse offers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37582/#review96397 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 5:39 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37582/ --- (Updated Aug. 25, 2015, 5:39 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3042 https://issues.apache.org/jira/browse/MESOS-3042 Repository: mesos Description --- Adds a test for the previous review in this chain. Diffs - src/tests/hierarchical_allocator_tests.cpp 9748ca0b3fee25dcec51c64d8ba84dbd4aaf Diff: https://reviews.apache.org/r/37582/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37747: Introduced bind-mount based provisioner Backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/#review96398 --- Patch looks great! Reviews applied: [37722, 37747] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 5:22 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/ --- (Updated Aug. 25, 2015, 5:22 p.m.) Review request for mesos, Lily Chen, Jie Yu, Timothy Chen, and Vinod Kone. Bugs: MESOS-3190 https://issues.apache.org/jira/browse/MESOS-3190 Repository: mesos Description --- Introduced bind-mount based provisioner Backend. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/backend.cpp 6190ce3eeff6ea22142c9eaa5a771ae1b767740c src/slave/containerizer/provisioners/backends/bind.hpp PRE-CREATION src/slave/containerizer/provisioners/backends/bind.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 src/tests/containerizer/provisioner_backend_tests.cpp PRE-CREATION src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 Diff: https://reviews.apache.org/r/37747/diff/ Testing --- sudo make check. Added one test. Thanks, Jiang Yan Xu
Re: Review Request 37747: Introduced bind-mount based provisioner Backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/#review96392 --- Patch looks great! Reviews applied: [37722, 37747] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 5:22 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/ --- (Updated Aug. 25, 2015, 5:22 p.m.) Review request for mesos, Lily Chen, Jie Yu, Timothy Chen, and Vinod Kone. Bugs: MESOS-3190 https://issues.apache.org/jira/browse/MESOS-3190 Repository: mesos Description --- Introduced bind-mount based provisioner Backend. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/backend.cpp 6190ce3eeff6ea22142c9eaa5a771ae1b767740c src/slave/containerizer/provisioners/backends/bind.hpp PRE-CREATION src/slave/containerizer/provisioners/backends/bind.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 src/tests/containerizer/provisioner_backend_tests.cpp PRE-CREATION src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 Diff: https://reviews.apache.org/r/37747/diff/ Testing --- sudo make check. Added one test. Thanks, Jiang Yan Xu
Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37180/#review96393 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37180/ --- (Updated Aug. 25, 2015, 2:13 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf Diff: https://reviews.apache.org/r/37180/diff/ Testing --- The tests break as expected. With the scheduler API change there are CHECKs that fail. Once we update the API these will be resolved. Thanks, Joris Van Remoortere
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
On Aug. 25, 2015, 2:27 p.m., Michael Park wrote:
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp, lines 142-172
https://reviews.apache.org/r/37187/diff/5/?file=1037650#file1037650line142
Given that there's no specified iteration order of a multihashmap, I
don't think this is even something to test for.
That is, this test doesn't test anything further than the test above it:
```
ASSERT_EQ(2u, map.get(foo).size());
ASSERT_TRUE(map.contains(foo, 1024));
ASSERT_TRUE(map.contains(foo, 1025));
```
I would say we should just remove it, what do you think?
Jan Schlicht wrote:
IMHO the purpose of this test is to show that the iterator iteratates
over all elements of a multihashmap, not that the iterater provides a
specific order. But given that that this is not too complicated it's probably
safe to remove this test.
Michael Park wrote:
Yeah, let's remove this test.
Michael Park wrote:
Hm, how about we actually leave the test but follow the pattern for the
`ForEach` test to ignore the ordering?
As the `ForEach` test shows that one can iterate over all elements of
multihashmap and would even fail if there where unexpected elements, it
includes the test for expected iterator behavior.
- Jan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/#review96331
---
On Aug. 25, 2015, 6:02 p.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 25, 2015, 6:02 p.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
Re: Review Request 37173: Maintenance Primitives: Added unavailability to Allocator's Slave struct.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37173/#review96323 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:12 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37173/ --- (Updated Aug. 25, 2015, 2:12 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a include/mesos/master/allocator.proto 10fd9a2d5fcbc18a9ca2d6c9c0ec1c605f21872b src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/hierarchical_allocator_tests.cpp 9748ca0b3fee25dcec51c64d8ba84dbd4aaf src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc Diff: https://reviews.apache.org/r/37173/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37362: Maintenance Primitives: Adds an endpoint for transitioning agents back into Normal mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37362/#review96313 --- src/master/maintenance.cpp (line 153) https://reviews.apache.org/r/37362/#comment151633 Not quite under what does the paramter strict means here? Seems it was not used. src/master/maintenance.cpp (line 157) https://reviews.apache.org/r/37362/#comment151634 I see that changed will be set to true when one machine removed from mantain list, what does changed means here? - Guangya Liu On Aug. 24, 2015, 7:08 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37362/ --- (Updated Aug. 24, 2015, 7:08 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2067 and MESOS-3069 https://issues.apache.org/jira/browse/MESOS-2067 https://issues.apache.org/jira/browse/MESOS-3069 Repository: mesos Description --- Endpoint: /maintenance.stop Registry operation = maintenance::StopMaintenance Sets the list of machines back to Normal mode. Removes those machines from the maintenance schedule. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 Diff: https://reviews.apache.org/r/37362/diff/ Testing --- `make check` New Tests: RegistrarTest.StopMaintenance Schedules some machines. Deactivates some. Reactivates some. MasterMaintenanceTest.ReactivateMachines Tests some invalid lists. Schedules some machines. Deactivates some. Tests some valid and invalid lists. Checks that the schedule is modified. Thanks, Joseph Wu
Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/#review96315
---
Ship it!
Ship It!
- Guangya Liu
On Aug. 24, 2015, 7:09 p.m., Joseph Wu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/
---
(Updated Aug. 24, 2015, 7:09 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan,
Joris Van Remoortere, and Vinod Kone.
Bugs: MESOS-2067
https://issues.apache.org/jira/browse/MESOS-2067
Repository: mesos
Description
---
Endpoint: /maintenance.status
Returns a JSON object like:
```
{
draining: [
{ hostname: foo, ip: 0.0.0.1 },
{ hostname: bar, ip: 0.0.0.2 },
],
deactivated: [
{ hostname: baz, ip: 0.0.0.3 },
]
}
```
Diffs
-
src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44
src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be
src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf
src/tests/master_maintenance_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37364/diff/
Testing
---
`make check`
New Tests:
MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
Extra test case for the /maintenance.schedule endpoint, which requires
all three endpoints to work.
MasterMaintenanceTest.MachineStatus
Schedules, starts, and stops maintenance. Checks machine statuses after
each step.
Thanks,
Joseph Wu
Re: Review Request 37170: Maintenance Primitives: Added `Machine` to Slave struct in Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37170/#review96316 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:12 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37170/ --- (Updated Aug. 25, 2015, 2:12 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf Diff: https://reviews.apache.org/r/37170/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37172/#review96317 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 2:12 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37172/ --- (Updated Aug. 25, 2015, 2:12 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/tests/master_maintenance_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37172/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96126 --- include/mesos/mesos.proto (line 123) https://reviews.apache.org/r/36321/#comment151360 Let's add a comment saying it's not a generic range, but a range relative to unavailability event. I think the reason you do not use a more generic name is that you plan to add specific fields in the future. If this is the case, let's reflect it in the comment for posterity. If it's not, let's rename the message for something more general : ). include/mesos/mesos.proto (line 129) https://reviews.apache.org/r/36321/#comment151359 For consistency, please one space between sentences! Here and below. include/mesos/mesos.proto (lines 847 - 850) https://reviews.apache.org/r/36321/#comment151361 As per my comment for `InverseOffers`, let's clarify the contract and guarantees here. - Alexander Rukletsov On Aug. 25, 2015, 3:24 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:24 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
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 `Period`? I'm thinking about a use case, when a custom allocator uses InverseOffers to ask a framework to release resources. In this case, we need 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. Joseph Wu wrote: A while ago, I posted a few diffs where this object was called `Interval` (https://reviews.apache.org/r/36321/diff/7/). The reason why it was changed back to `Unavailability` is that we may wish to extend this object to be more specific, in the future. (We've already removed all the maintenance-specific language in the comments for `Unavailability` and `InverseOffer`.) Taking your example, the custom allocator asks for resources back. It says that these will be unavailable by the `start` time. Duration is optional; in the case of maintenance, when `duration` is omitted, it means the duration is forever or unknown. I think the term also works for non-maintenance uses. Alexander Rukletsov wrote: For me unavailability implies the resources will be given back once the period (interval) is over. Unless resource are reserved, this is not the case, since allocator has no obligations to offer resources to prior users once unavailability period is over. In an offline conversation, Joris pointed out, that unavailability events are mostly interesting for stateful frameworks, which most probably will have reservations for resources. If you plan to leave current term, could you please reflect in the comment what unavailability guarantees and what it does not? Joseph Wu wrote: Updated the comments. Let me know what you think. I think the comment is great: brief and clear. One thing I'm not sure about is whether it should be placed in the `InverseOffer` message, since there is a similar field in `ResourceOffer`. Maybe it makes sense to pull it up to the `Unavailability` definition and leave a reference to it in both places, where `unavailability` is used. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96073 --- On Aug. 25, 2015, 3:24 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:24 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
On Aug. 25, 2015, 9:55 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, lines 847-850 https://reviews.apache.org/r/36321/diff/9/?file=1038857#file1038857line847 As per my comment for `InverseOffers`, let's clarify the contract and guarantees here. Or, as I said earlier, we can pull the contarct description out and reference it here. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96126 --- On Aug. 25, 2015, 3:24 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:24 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96421 --- src/Makefile.am (line 477) https://reviews.apache.org/r/37427/#comment151767 Group token_manager.hpp with other provisioner header files, see lines 740-744. src/slave/containerizer/provisioners/docker/token_manager.hpp (lines 22 - 23) https://reviews.apache.org/r/37427/#comment151768 Make sure included libraries are in alphabetical order. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 26) https://reviews.apache.org/r/37427/#comment151769 Same here (libraries in alphabetical order). src/slave/containerizer/provisioners/docker/token_manager.hpp (lines 30 - 33) https://reviews.apache.org/r/37427/#comment151770 Stout libraries are usually included before process libraries src/slave/containerizer/provisioners/docker/token_manager.hpp (line 57) https://reviews.apache.org/r/37427/#comment151771 Name parameters. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 96) https://reviews.apache.org/r/37427/#comment151773 Period at the end of comment. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 97) https://reviews.apache.org/r/37427/#comment151772 classes are separated by two empty lines. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 198) https://reviews.apache.org/r/37427/#comment151774 name parameter. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 213) https://reviews.apache.org/r/37427/#comment151797 place member variables after function declarations. src/slave/containerizer/provisioners/docker/token_manager.cpp (lines 57 - 70) https://reviews.apache.org/r/37427/#comment151776 Why not just use a private member function? src/slave/containerizer/provisioners/docker/token_manager.cpp (lines 217 - 219) https://reviews.apache.org/r/37427/#comment151779 Should only be indented 4 spaces. src/slave/containerizer/provisioners/docker/token_manager.cpp (lines 249 - 251) https://reviews.apache.org/r/37427/#comment151792 Indent 2 more spaces src/slave/containerizer/provisioners/docker/token_manager.cpp (lines 254 - 255) https://reviews.apache.org/r/37427/#comment151793 Indent by 2 spaces src/slave/containerizer/provisioners/docker/token_manager.cpp (line 318) https://reviews.apache.org/r/37427/#comment151786 Capitalize beginning of failure messages. src/slave/containerizer/provisioners/docker/token_manager.cpp (line 321) https://reviews.apache.org/r/37427/#comment151790 indent 2 more spaces? src/slave/containerizer/provisioners/docker/token_manager.cpp (line 323) https://reviews.apache.org/r/37427/#comment151787 Here too. src/slave/containerizer/provisioners/docker/token_manager.cpp (line 330) https://reviews.apache.org/r/37427/#comment151789 here too. src/slave/containerizer/provisioners/docker/token_manager.cpp (line 341) https://reviews.apache.org/r/37427/#comment151791 Fix implementatio typo and period at the end of comment. src/tests/provisioners/docker_provisioner_tests.cpp (line 562) https://reviews.apache.org/r/37427/#comment151799 extra space here. - Lily Chen On Aug. 24, 2015, 5:16 p.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 24, 2015, 5:16 p.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37497: Added Docker provisioner paths which handles path manipulation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37497/#review96433 --- Patch looks great! Reviews applied: [37196, 37197, 37198, 37200, 37247, 37495, 37496, 37497] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 9:03 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37497/ --- (Updated Aug. 25, 2015, 9:03 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added Docker provisioner paths which handles path manipulation. Diffs - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37497/diff/ Testing --- sudo make check ./bin/mesos-tests.sh --gtest_filter=*DockerProvisioner* --gtest_repeat=20 --gtest_shuffle=1 Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
On Aug. 25, 2015, 9:30 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 104 https://reviews.apache.org/r/37114/diff/5/?file=1051861#file1051861line104 prefixit sounds wierd, how about prefixSocket? Will change On Aug. 25, 2015, 9:30 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 106 https://reviews.apache.org/r/37114/diff/5/?file=1051861#file1051861line106 Let's make these constants. will change On Aug. 25, 2015, 9:30 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 107 https://reviews.apache.org/r/37114/diff/5/?file=1051861#file1051861line107 Btw I think you should just use strings::startswith, same for all other checks. I don't know of method strings::startswith in c++ .. .but would change it to compare if that's okay ... - Vaibhav --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96424 --- On Aug. 25, 2015, 9:29 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37722: Added definitions of container rootfs directories.
On Aug. 24, 2015, 5:20 p.m., Timothy Chen wrote: src/slave/containerizer/provisioners/appc/paths.hpp, line 55 https://reviews.apache.org/r/37722/diff/2/?file=1048535#file1048535line55 Suggestion: It might make sense to nest one more directory rootfs so you can add metadata about that rootfs besides it, also allow more flexibility in the long run. Thanks Tim! I chose to use a parent rootfses dir to address this concern. This is more inline with current practices: Create a parent dir and use a sibling file to store the metadata. e.g.: for ``` |-- backend (copy, bind, etc.) |-- rootfses |-- XYZ (the rootfs) ``` The meta file could be ``` |-- backend (copy, bind, etc.) |-- rootfses |-- XYZ (the meta file) ``` **(Under the meta dir!)**, but of course we'll sort this out when there is an actual use case. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96252 --- On Aug. 24, 2015, 11:26 a.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 11:26 a.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 36908: Added QuotaInfo Protobuf.
On Aug. 25, 2015, 5:51 a.m., Guangya Liu wrote: include/mesos/master/quota.proto, line 19 https://reviews.apache.org/r/36908/diff/4/?file=1032679#file1032679line19 Yes, does v1 API will be supportted for quota? Adding new protobufs should be backwards-compatible, so I suppose the answer is yes. On Aug. 25, 2015, 5:51 a.m., Guangya Liu wrote: src/Makefile.am, line 182 https://reviews.apache.org/r/36908/diff/4/?file=1032680#file1032680line182 It is better consider the alpha order of those files, we can add quota related files under module related files. Could you please elaborate? I'm not sure I follow. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review96299 --- On Aug. 5, 2015, 2:03 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated Aug. 5, 2015, 2:03 p.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/ --- (Updated Aug. 25, 2015, 8:19 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3240 https://issues.apache.org/jira/browse/MESOS-3240 Repository: mesos Description --- Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too. Diffs (updated) - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86 src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37500/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 25, 2015, 8:57 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- (Updated Aug. 25, 2015, 8:59 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs (updated) - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37247: Added Docker image reference store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37247/ --- (Updated Aug. 25, 2015, 9 p.m.) Review request for mesos and Timothy Chen. Changes --- Rebased on master. Bugs: MESOS-3021 https://issues.apache.org/jira/browse/MESOS-3021 Repository: mesos Description --- Added Docker image reference store. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/messages/docker_provisioner.hpp PRE-CREATION src/messages/docker_provisioner.proto PRE-CREATION src/slave/containerizer/provisioners/docker/reference_store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37247/diff/ Testing --- make check Tests will be added in a later review. Thanks, Lily Chen
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/ --- (Updated Aug. 25, 2015, 8:25 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3240 https://issues.apache.org/jira/browse/MESOS-3240 Repository: mesos Description --- Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too. Diffs (updated) - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86 src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37500/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 37495: Docker provisioner local store unit tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37495/ --- (Updated Aug. 25, 2015, 9:01 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Docker provisioner local store unit tests. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37495/diff/ Testing --- sudo make check ./bin/mesos-tests.sh --gtest_filter=*DockerProvisioner* --gtest_repeat=20 --gtest_shuffle=1 Thanks, Lily Chen
Re: Review Request 37496: Move docker provisioner local store into dedicated folders.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37496/ --- (Updated Aug. 25, 2015, 9:02 p.m.) Review request for mesos and Timothy Chen. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Move docker provisioner local store into dedicated folders. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37496/diff/ Testing --- sudo make check ./bin/mesos-tests.sh --gtest_filter=*DockerProvisioner* --gtest_repeat=20 --gtest_shuffle=1 Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
On Aug. 25, 2015, 5:05 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 102 https://reviews.apache.org/r/37114/diff/4/?file=1047856#file1047856line102 Also does this mean if a user put in tcp:/// we're just padding unix:///? I think it's safe to assume we should always expect a absolute path to the socket, and I'm not sure what other prefix are we allowing here by allowing a custom prefix to be passed in. Vaibhav Khanduja wrote: The default value of the flag does not come with absolute path and changing would break the logic of slaves containerization as we discussed ealier (flags.hpp). The code checks for prefix presence before adding it. I can make this check for other values - tcp:// and fd://. If no prefix is found, I would assume no value at command line is provided and the default value is flags.hpp is being passed, thus add unix:// prefix. Let me know if you are okay with solution? I now check for all three - Vaibhav --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96360 --- On Aug. 25, 2015, 7:27 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 7:27 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler failover
On Aug. 24, 2015, 4:56 p.m., Vinod Kone wrote: This review is a bit hard to follow because it's doing multiple things. I would recommend you to split this into multiple reviews #1) Expose framework user in state.json #2) Update framework user on re-registration (need a test for this!) Aditi Dixit wrote: Added the test. How do I split this into multiple reviews? Discard this one and open two new ones by shifting the code into new branches and then opening review requests? You could: ``` # To update this review into the first in a review chain: git branch temp # Save your work. Delete part of your changes # i.e. the re-registration stuff. git add --update git commit --amend # For the second review: git checkout temp -- . # Restore everything you deleted into a second commit. git commit support/post-reviews.py ``` - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/#review96240 --- On Aug. 25, 2015, 1:25 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/ --- (Updated Aug. 25, 2015, 1:25 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3240 https://issues.apache.org/jira/browse/MESOS-3240 Repository: mesos Description --- Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86 src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37500/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 37497: Added Docker provisioner paths which handles path manipulation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37497/ --- (Updated Aug. 25, 2015, 9:03 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added Docker provisioner paths which handles path manipulation. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37497/diff/ Testing --- sudo make check ./bin/mesos-tests.sh --gtest_filter=*DockerProvisioner* --gtest_repeat=20 --gtest_shuffle=1 Thanks, Lily Chen
Re: Review Request 36868: Added - operators for Option, Try, Result.
On July 29, 2015, 4:42 a.m., Michael Park wrote: Why wasn't [r36869](https://reviews.apache.org/r/36869) just included in this patch? Ah, it's because this patch is `stout` whereas [r36869](https://reviews.apache.org/r/36869/) is `libprocess`. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36868/#review93388 --- On July 28, 2015, 12:56 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36868/ --- (Updated July 28, 2015, 12:56 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere. Bugs: MESOS-2757 https://issues.apache.org/jira/browse/MESOS-2757 Repository: mesos Description --- See MESOS-2757. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36868/diff/ Testing --- Added tests. Thanks, Ben Mahler
Re: Review Request 37198: Add Docker image provisioner and copy backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/ --- (Updated Aug. 25, 2015, 8:58 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Changes --- Rebased on master. Bugs: MESOS-2850 https://issues.apache.org/jira/browse/MESOS-2850 Repository: mesos Description --- Add Docker image provisioner and copy backend. Diffs (updated) - src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab src/slave/containerizer/isolators/filesystem/linux.cpp f36424e94c380870cfde49d55af397fa3dc4a612 src/slave/containerizer/provisioner.hpp 541dd4e0b2f0c92a45c00cab6132a2be69654838 src/slave/containerizer/provisioner.cpp efc7e6996ff6663bebaf61989a7e040bd2ad7a5e src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 src/tests/containerizer/provisioner.hpp c4ba46794fe5d7875fda11155367f521c34ea339 Diff: https://reviews.apache.org/r/37198/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37747: Introduced bind-mount based provisioner Backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37747/ --- (Updated Aug. 25, 2015, 5:19 p.m.) Review request for mesos, Lily Chen, Jie Yu, Timothy Chen, and Vinod Kone. Changes --- Comments. NNFR. Bugs: MESOS-3190 https://issues.apache.org/jira/browse/MESOS-3190 Repository: mesos Description --- Introduced bind-mount based provisioner Backend. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/backend.hpp 46120e8420cc491a0decbd88301f89d6dfcff120 src/slave/containerizer/provisioners/backend.cpp 6190ce3eeff6ea22142c9eaa5a771ae1b767740c src/slave/containerizer/provisioners/backends/bind.hpp PRE-CREATION src/slave/containerizer/provisioners/backends/bind.cpp PRE-CREATION src/tests/containerizer/provisioner_backend_tests.cpp PRE-CREATION src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 Diff: https://reviews.apache.org/r/37747/diff/ Testing --- sudo make check. Added one test. Thanks, Jiang Yan Xu
Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/#review96301 --- include/mesos/maintenance/maintenance.proto (line 19) https://reviews.apache.org/r/36571/#comment151624 I think that you will handle v1 api in other patches? include/mesos/mesos.proto (line 111) https://reviews.apache.org/r/36571/#comment151623 Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine. - Guangya Liu On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36571/ --- (Updated Aug. 24, 2015, 6:33 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3066 https://issues.apache.org/jira/browse/MESOS-3066 Repository: mesos Description --- * MachineInfo - Describes a single box that holds one or more agents. * MachineInfos - A list of boxes. * maintenance::Window - A set of machines and a planned downtime period. * maintenance::Schedule - A set of maintenance windows. * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated. * Registry::MaintenanceStatus - Holds the maintenance mode of a machine. Diffs - include/mesos/maintenance/maintenance.hpp PRE-CREATION include/mesos/maintenance/maintenance.proto PRE-CREATION include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 Diff: https://reviews.apache.org/r/36571/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37188: Added std::hash template specializations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37188/ --- (Updated Aug. 25, 2015, 3:15 p.m.) Review request for mesos, Alexander Rojas and Michael Park. Changes --- Rebase master fix issue. Bugs: MESOS-3217 https://issues.apache.org/jira/browse/MESOS-3217 Repository: mesos Description --- Added std::hash template specializations. Diffs (updated) - 3rdparty/libprocess/include/process/address.hpp df78f8e525c40b87e734e16979d3315f89e12594 3rdparty/libprocess/include/process/pid.hpp 8d3735c7d5b8f74a7a0ebb8cafe7c7ebee68f5f0 3rdparty/libprocess/src/pid.cpp f5528aea04a17ce868a0f67b94defdefea18e234 3rdparty/libprocess/src/tests/http_tests.cpp c22f9d35b5dc959ce9816d3358ebff92b853bf52 Diff: https://reviews.apache.org/r/37188/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 37189: Added std::hash template specializations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37189/ --- (Updated Aug. 25, 2015, 3:16 p.m.) Review request for mesos, Alexander Rojas and Michael Park. Changes --- Rebase master Bugs: MESOS-3217 https://issues.apache.org/jira/browse/MESOS-3217 Repository: mesos Description --- Added std::hash template specializations. Diffs (updated) - include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/linux/cgroups.hpp 91ccfd046256d29504e93003de83c04f048c9b78 src/master/metrics.hpp c51887ee1e072a063d629a735a3c6a2212e616ef src/messages/log.hpp 94e38c189703ff22c412b86429e052e942e75e75 src/module/manager.hpp cab67a8b1ee7164bb3cb2a8b24e1a6e05b40fd19 src/module/manager.cpp 862b71fa9d1e303e98de3767d69c9805e7b26dcc src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 src/tests/fetcher_tests.cpp 81e70368f7a6164e9649964881aa5a688ef222e8 src/tests/module.hpp 850c2002882be52e25f191f17785698deb5513c1 Diff: https://reviews.apache.org/r/37189/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 37281: Maintenance Primitives: Added Unavailability to Offer in V1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37281/ --- (Updated Aug. 25, 2015, 10:48 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37281/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:53 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Commenting changes and clarifications. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description (updated) --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. Also copied to v1 API. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs (updated) - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
On Aug. 25, 2015, 2:55 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, line 129 https://reviews.apache.org/r/36321/diff/9/?file=1038857#file1038857line129 For consistency, please one space between sentences! Here and below. The codebase isn't consistent in this respect. But grammatically, 2 spaces is correct (notice that the ASF license text uses 2 spaces). On Aug. 25, 2015, 2:55 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, lines 847-850 https://reviews.apache.org/r/36321/diff/9/?file=1038857#file1038857line847 As per my comment for `InverseOffers`, let's clarify the contract and guarantees here. Alexander Rukletsov wrote: Or, as I said earlier, we can pull the contarct description out and reference it here. I actually left it out on purpose. The Unavailability in an Offer is a nugget of extra information about the resources. The presence of the unavailability shouldn't drastically alter how frameworks perceive the offer; an offer is still meant to be an allocation. If we put too much of the contract/guarantee logic into the offer, it'll start sounding like an offer for deallocation (that's what the InverseOffer is for). On Aug. 25, 2015, 2:55 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, line 123 https://reviews.apache.org/r/36321/diff/9/?file=1038857#file1038857line123 Let's add a comment saying it's not a generic range, but a range relative to unavailability event. I think the reason you do not use a more generic name is that you plan to add specific fields in the future. If this is the case, let's reflect it in the comment for posterity. If it's not, let's rename the message for something more general : ). I'll add a comment *and* a TODO :) - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96126 --- On Aug. 25, 2015, 3:53 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:53 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. Also copied to v1 API. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.
On Aug. 25, 2015, 10:21 a.m., Guangya Liu wrote: include/mesos/mesos.proto, line 939 https://reviews.apache.org/r/36321/diff/10/?file=1048529#file1048529line939 Thanks Joe! Got it, but I think that it is better that we can add some notes here to be more clear Next time, please continue the discussion thread (and re-open the issue) instead of adding a new comment. I'll expand on the sentence a bit. - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/#review96366 --- On Aug. 25, 2015, 3:53 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36321/ --- (Updated Aug. 25, 2015, 3:53 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2061 and MESOS-2066 https://issues.apache.org/jira/browse/MESOS-2061 https://issues.apache.org/jira/browse/MESOS-2066 Repository: mesos Description --- MESOS-2061: Add Unavailability and InverseOffer protobufs declarations. MESOS-2066: Add the Unavailability field to Offers. Also copied to v1 API. No integration with other components (that part is tracked in separate JIRAs, see MESOS-1474). Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/36321/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96449 --- src/docker/docker.cpp (line 111) https://reviews.apache.org/r/37114/#comment151830 This should be comparison of ! with 0 src/docker/docker.cpp (line 116) https://reviews.apache.org/r/37114/#comment151831 This should be comparison of ! with 0 src/docker/docker.cpp (line 121) https://reviews.apache.org/r/37114/#comment151833 This should be comparison of ! with 0 - Vaibhav Khanduja On Aug. 25, 2015, 10:53 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 10:53 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37282: Maintenance Primitives: Added InverseOffer to V1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37282/ --- (Updated Aug. 25, 2015, 10:48 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu. Bugs: MESOS-1474 https://issues.apache.org/jira/browse/MESOS-1474 Repository: mesos Description --- See summary. Diffs - include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 src/internal/devolve.hpp 6e4306df78b9b8d2054e6550209341fd7b0972d6 src/internal/devolve.cpp 0a069e51053b572a8d5dc95380732119504dd0c9 src/internal/evolve.hpp 13e9f52da98567038ec717f394f79e526a1521e9 src/internal/evolve.cpp 11ce9e77490e93f781ceebc33063d13953a11765 src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c Diff: https://reviews.apache.org/r/37282/diff/ Testing --- Thanks, Joris Van Remoortere
Re: Review Request 37532: Add QUIESCE call interface to the scheduler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37532/#review96453 --- Can you split the addition of suppress to the driver (sched.cpp) and http library (scheduler.cpp) into separate patches? Note that the former needs updates to the Java and Python bindings as well. More importantly, are you planning to plumb this call through to the allocator to actually do the quiescing/suppressing? - Vinod Kone On Aug. 22, 2015, 2:27 p.m., Guangya Liu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37532/ --- (Updated Aug. 22, 2015, 2:27 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3037 https://issues.apache.org/jira/browse/MESOS-3037 Repository: mesos Description --- This is just part of MESOS-3037, this patch only add the interface of QUIESCE call. Diffs - include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 include/mesos/v1/scheduler/scheduler.proto bd5e82a614b1163b29f9b20e562208efa1ba4b55 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 Diff: https://reviews.apache.org/r/37532/diff/ Testing --- Thanks, Guangya Liu
Re: Review Request 37500: Update the FrameworkInfo.user on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/#review96422 --- Patch looks great! Reviews applied: [37500] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 8:25 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37500/ --- (Updated Aug. 25, 2015, 8:25 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3240 https://issues.apache.org/jira/browse/MESOS-3240 Repository: mesos Description --- Added user to master's state, slave's state (not exposed) and updated user in all slaves that have the registered framework. Added the test too. Diffs - src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86 src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 Diff: https://reviews.apache.org/r/37500/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Changes --- Fixed intentation issues Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs (updated) - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96414 --- src/docker/docker.cpp (line 104) https://reviews.apache.org/r/37114/#comment151759 lowerCamelCase function name src/docker/docker.cpp (lines 682 - 683) https://reviews.apache.org/r/37114/#comment151752 Intended with two spaces from continuation statement src/docker/executor.cpp (line 409) https://reviews.apache.org/r/37114/#comment151757 Intend with two spaces src/tests/containerizer/docker_containerizer_tests.cpp (line 242) https://reviews.apache.org/r/37114/#comment151763 Intent with two spaces src/tests/containerizer/docker_containerizer_tests.cpp (line 424) https://reviews.apache.org/r/37114/#comment151765 Intent with two spaces ...and rest of the code - Vaibhav Khanduja On Aug. 25, 2015, 9:29 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96424 --- src/docker/docker.cpp (line 104) https://reviews.apache.org/r/37114/#comment151781 prefixit sounds wierd, how about prefixSocket? src/docker/docker.cpp (line 106) https://reviews.apache.org/r/37114/#comment151778 Let's make these constants. src/docker/docker.cpp (line 107) https://reviews.apache.org/r/37114/#comment151780 Btw I think you should just use strings::startswith, same for all other checks. - Timothy Chen On Aug. 25, 2015, 9:29 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 25, 2015, 9:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 36868: Added - operators for Option, Try, Result.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36868/#review96429 --- Ship it! Adjust the operator spacing. Keeping the current style for now, we can align it differently later if we want. I'd rather get the functionality in. Using the recommended way to delegate from the non-const version to the const version. - Joris Van Remoortere On July 28, 2015, 12:56 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36868/ --- (Updated July 28, 2015, 12:56 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere. Bugs: MESOS-2757 https://issues.apache.org/jira/browse/MESOS-2757 Repository: mesos Description --- See MESOS-2757. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36868/diff/ Testing --- Added tests. Thanks, Ben Mahler
Re: Review Request 36869: Added try_tests.cpp to libprocess makefile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36869/#review96430 --- Ship it! Ship It! - Joris Van Remoortere On July 28, 2015, 12:56 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36869/ --- (Updated July 28, 2015, 12:56 a.m.) Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere. Bugs: MESOS-2757 https://issues.apache.org/jira/browse/MESOS-2757 Repository: mesos Description --- See MESOS-2757. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd95fe197532131255072a86aba83cd5e822419a Diff: https://reviews.apache.org/r/36869/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 37187: Use std::unordered_{set, map} instead of boost::unordered_{set, map}.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/#review96331
---
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp (lines 142 - 172)
https://reviews.apache.org/r/37187/#comment151644
Given that there's no specified iteration order of a multihashmap, I don't
think this is even something to test for.
That is, this test doesn't test anything further than the test above it:
```
ASSERT_EQ(2u, map.get(foo).size());
ASSERT_TRUE(map.contains(foo, 1024));
ASSERT_TRUE(map.contains(foo, 1025));
```
I would say we should just remove it, what do you think?
- Michael Park
On Aug. 11, 2015, 11:57 a.m., Jan Schlicht wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37187/
---
(Updated Aug. 11, 2015, 11:57 a.m.)
Review request for mesos, Alexander Rojas and Michael Park.
Bugs: MESOS-3217
https://issues.apache.org/jira/browse/MESOS-3217
Repository: mesos
Description
---
Changed hashmap, hashset to use std::unordered_{set,map} instead of
boost::unordered_{set,map}.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp
25684a405bfa9c4ab65641568341652a8efaf925
3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp
ecab60a21765c58b0732de747509aa6382d31c06
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
2dd1905a4626a7d7e9f61863c8290ae1cdb9b925
3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp
a8573ed67e20b5206afd69bab4f5dc094a7e882f
3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp
e8ebe0b2f5e49657ee191a2535e0abdaf8e665ce
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3802a29b82da57217dd75c6b1611fd21c91cfc03
3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp
b625ffaeb3672f58fbd9558a868f87404e659c53
Diff: https://reviews.apache.org/r/37187/diff/
Testing
---
make check
Thanks,
Jan Schlicht
