Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review93863 --- Bad patch! Reviews applied: [35797] Failed command: ./support/apply-review.sh -n -r 35797 Error: 2015-08-02 17:48:08 URL:https://reviews.apache.org/r/35797/diff/raw/ [11086/11086] - 35797.patch [1] error: patch failed: src/master/allocator/mesos/hierarchical.hpp:486 error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply error: patch failed: src/master/master.hpp:530 error: src/master/master.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 2, 2015, 4:32 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated Aug. 2, 2015, 4:32 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This updates both the master as well as the allocator. Added test for both. Diffs - src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e src/tests/oversubscription_tests.cpp c7a2dacb600d7703de6090e7e47f453a3d08b53a Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review93200 --- Ship it! Looks good modulo some minor indentation issues. I will fix them for you and commit. src/tests/fault_tolerance_tests.cpp (line 1833) https://reviews.apache.org/r/35797/#comment147517 bad indentation. src/tests/fault_tolerance_tests.cpp (line 1837) https://reviews.apache.org/r/35797/#comment147518 bad indentation. src/tests/fault_tolerance_tests.cpp (line 1842) https://reviews.apache.org/r/35797/#comment147519 bad indentatation. src/tests/fault_tolerance_tests.cpp (line 1851) https://reviews.apache.org/r/35797/#comment147520 bad indetation. - Vinod Kone On July 25, 2015, 6:38 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 25, 2015, 6:38 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This updates both the master as well as the allocator. Added test for master. Diffs - src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review93035 --- Patch looks great! Reviews applied: [35797] All tests passed. - Mesos ReviewBot On July 25, 2015, 6:38 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 25, 2015, 6:38 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This updates both the master as well as the allocator. Added test for master. Diffs - src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 25, 2015, 6:38 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description (updated) --- This updates both the master as well as the allocator. Added test for master. Diffs (updated) - src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
On July 16, 2015, 8:13 p.m., Vinod Kone wrote: src/tests/fault_tolerance_tests.cpp, line 1831 https://reviews.apache.org/r/35797/diff/3/?file=1013501#file1013501line1831 Nice test! Thanks :) On July 16, 2015, 8:13 p.m., Vinod Kone wrote: src/master/master.hpp, line 534 https://reviews.apache.org/r/35797/diff/3/?file=1013500#file1013500line534 s/capabilities_size()/capabilities.size()/ In mesos.pb.h, capabilities_size() is defined in line 1147 :) - Aditi --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review91955 --- On July 18, 2015, 2:23 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 18, 2015, 2:23 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This only updates the master, not the allocator. Added test too. Diffs - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 18, 2015, 2:23 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This only updates the master, not the allocator. Added test too. Diffs (updated) - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review91948 --- Patch looks great! Reviews applied: [35797] All tests passed. - Mesos ReviewBot On July 16, 2015, 6:46 p.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 16, 2015, 6:46 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- This only updates the master, not the allocator. Added test too. Diffs - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated July 16, 2015, 6:46 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description (updated) --- This only updates the master, not the allocator. Added test too. Diffs (updated) - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35797/#review91955
---
src/master/master.hpp (line 534)
https://reviews.apache.org/r/35797/#comment145723
s/capabilities_size()/capabilities.size()/
src/master/master.hpp (line 536)
https://reviews.apache.org/r/35797/#comment145722
Do you also want to deal with the case where capabilities are previously
set but now being unset? In other words, the else case.
if (source.capabilities.size() 0 ) {
info.mutable_capabilities()-CopyFrom(source.capabilities);
} else {
info.clear_capabilities();
}
If you want do this in a follow up patch, add a TODO here. You will also
need a new test for this.
src/tests/fault_tolerance_tests.cpp (line 1730)
https://reviews.apache.org/r/35797/#comment145724
Add a comment on the top of the test that you are verifying that when a
framework re-registers with updated framework info, it gets updated in the
master.
src/tests/fault_tolerance_tests.cpp (lines 1742 - 1743)
https://reviews.apache.org/r/35797/#comment145720
We now require a much newer gcc version, so this shouldn't be an issue. You
can just do.
FrameworkInfo framework1 = DEFAULT_FRAMEWORK_INFO;
src/tests/fault_tolerance_tests.cpp (lines 1769 - 1770)
https://reviews.apache.org/r/35797/#comment145725
ditto. just assign on the same line.
src/tests/fault_tolerance_tests.cpp (line 1772)
https://reviews.apache.org/r/35797/#comment145726
s/Name/Type/
src/tests/fault_tolerance_tests.cpp (lines 1773 - 1774)
https://reviews.apache.org/r/35797/#comment145730
Can you also update webui_url, hostname and failover_timeout while you are
at it?
src/tests/fault_tolerance_tests.cpp (line 1803)
https://reviews.apache.org/r/35797/#comment145732
s/masterState/response/
src/tests/fault_tolerance_tests.cpp (line 1806)
https://reviews.apache.org/r/35797/#comment145733
s/masterStateObject/parse/
you also need a
ASSERT_SOME(parse);
src/tests/fault_tolerance_tests.cpp (line 1809)
https://reviews.apache.org/r/35797/#comment145734
new line after ASSERT.
s/masterStateObject_/state/
src/tests/fault_tolerance_tests.cpp (line 1812)
https://reviews.apache.org/r/35797/#comment145736
s/famework_/framework/
new line after this.
src/tests/fault_tolerance_tests.cpp (line 1831)
https://reviews.apache.org/r/35797/#comment145737
Nice test!
- Vinod Kone
On July 16, 2015, 6:46 p.m., Aditi Dixit wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35797/
---
(Updated July 16, 2015, 6:46 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-2880
https://issues.apache.org/jira/browse/MESOS-2880
Repository: mesos
Description
---
This only updates the master, not the allocator. Added test too.
Diffs
-
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/tests/fault_tolerance_tests.cpp
1070ccf17f98f6b3800684a5edd6517d90631c3e
Diff: https://reviews.apache.org/r/35797/diff/
Testing
---
make check
Thanks,
Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated June 24, 2015, 7:14 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description (updated) --- This only updates the master, not the allocator. Diffs - src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/ --- (Updated June 24, 2015, 7:13 p.m.) Review request for mesos and Vinod Kone. Summary (updated) - Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities Diffs - src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 Diff: https://reviews.apache.org/r/35797/diff/ Testing --- make check Thanks, Aditi Dixit
