You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.io> on 2019/02/06 21:02:48 UTC
Review Request 69910: Added tests for feedback for operations on agent
default resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/
-----------------------------------------------------------
Review request for mesos, Greg Mann and Joseph Wu.
Bugs: MESOS-9473
https://issues.apache.org/jira/browse/MESOS-9473
Repository: mesos
Description
-------
Added tests for feedback for operations on agent default resources.
Diffs
-----
src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69910/diff/1/
Testing
-------
`bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
Thanks,
Gastón Kleiman
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/#review212650
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Feb. 7, 2019, 9:53 p.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69910/
> -----------------------------------------------------------
>
> (Updated Feb. 7, 2019, 9:53 p.m.)
>
>
> Review request for mesos, Greg Mann and Joseph Wu.
>
>
> Bugs: MESOS-9473
> https://issues.apache.org/jira/browse/MESOS-9473
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added tests for feedback for operations on agent default resources.
>
>
> Diffs
> -----
>
> src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
> src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
> src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69910/diff/2/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/
-----------------------------------------------------------
(Updated Feb. 8, 2019, 1:27 p.m.)
Review request for mesos, Greg Mann and Joseph Wu.
Changes
-------
Fixed compilation issue with clang 3.9.x
Bugs: MESOS-9473
https://issues.apache.org/jira/browse/MESOS-9473
Repository: mesos
Description
-------
Added tests for feedback for operations on agent default resources.
Diffs (updated)
-----
src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69910/diff/3/
Changes: https://reviews.apache.org/r/69910/diff/2-3/
Testing
-------
`bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
Thanks,
Gastón Kleiman
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
> On Feb. 7, 2019, 4:32 p.m., Greg Mann wrote:
> > src/tests/agent_operation_feedback_tests.cpp
> > Lines 179 (patched)
> > <https://reviews.apache.org/r/69910/diff/2/?file=2124478#file2124478line179>
> >
> > We should probably advance the clock by a multiple of this, like Benno was doing in his test. That way, if the acknowledgement is not processed correctly, we would be sure to hit the retry, since the agent backs off as it retries.
Benno's test advances the clock by a multiple of the minimum interval, this test advances it by the maximum interval. I manually verified before posting the updated patch that commenting out the ack call triggers another retry and makes the test fail.
- Gastón
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/#review212651
-----------------------------------------------------------
On Feb. 7, 2019, 1:53 p.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69910/
> -----------------------------------------------------------
>
> (Updated Feb. 7, 2019, 1:53 p.m.)
>
>
> Review request for mesos, Greg Mann and Joseph Wu.
>
>
> Bugs: MESOS-9473
> https://issues.apache.org/jira/browse/MESOS-9473
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added tests for feedback for operations on agent default resources.
>
>
> Diffs
> -----
>
> src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
> src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
> src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69910/diff/2/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/#review212651
-----------------------------------------------------------
src/tests/agent_operation_feedback_tests.cpp
Lines 179 (patched)
<https://reviews.apache.org/r/69910/#comment298499>
We should probably advance the clock by a multiple of this, like Benno was doing in his test. That way, if the acknowledgement is not processed correctly, we would be sure to hit the retry, since the agent backs off as it retries.
- Greg Mann
On Feb. 7, 2019, 9:53 p.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69910/
> -----------------------------------------------------------
>
> (Updated Feb. 7, 2019, 9:53 p.m.)
>
>
> Review request for mesos, Greg Mann and Joseph Wu.
>
>
> Bugs: MESOS-9473
> https://issues.apache.org/jira/browse/MESOS-9473
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added tests for feedback for operations on agent default resources.
>
>
> Diffs
> -----
>
> src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
> src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
> src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69910/diff/2/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/
-----------------------------------------------------------
(Updated Feb. 7, 2019, 1:53 p.m.)
Review request for mesos, Greg Mann and Joseph Wu.
Changes
-------
Addressed feedback and changed `STATUS_UPDATE_RETRY_INTERVAL_MIN` for `STATUS_UPDATE_RETRY_INTERVAL_MAX` in a test.
Bugs: MESOS-9473
https://issues.apache.org/jira/browse/MESOS-9473
Repository: mesos
Description
-------
Added tests for feedback for operations on agent default resources.
Diffs (updated)
-----
src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69910/diff/2/
Changes: https://reviews.apache.org/r/69910/diff/1-2/
Testing
-------
`bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
Thanks,
Gastón Kleiman
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
> On Feb. 7, 2019, 9:24 a.m., Greg Mann wrote:
> > I just realized this test is doing the same thing as `MasterAPITest.OperationFeedbackOnAgentDefaultResources`, maybe we should just move that one to your new file? Or remove that one and retain this one?
Removing that test in https://reviews.apache.org/r/69920/.
> On Feb. 7, 2019, 9:24 a.m., Greg Mann wrote:
> > src/tests/agent_operation_feedback_tests.cpp
> > Lines 156 (patched)
> > <https://reviews.apache.org/r/69910/diff/1/?file=2124147#file2124147line156>
> >
> > Is this necessary?
The test passes without it, so it is not necessary =).
- Gastón
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/#review212631
-----------------------------------------------------------
On Feb. 6, 2019, 1:02 p.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69910/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2019, 1:02 p.m.)
>
>
> Review request for mesos, Greg Mann and Joseph Wu.
>
>
> Bugs: MESOS-9473
> https://issues.apache.org/jira/browse/MESOS-9473
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added tests for feedback for operations on agent default resources.
>
>
> Diffs
> -----
>
> src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
> src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
> src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69910/diff/1/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 69910: Added tests for feedback for operations on
agent default resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69910/#review212631
-----------------------------------------------------------
I just realized this test is doing the same thing as `MasterAPITest.OperationFeedbackOnAgentDefaultResources`, maybe we should just move that one to your new file? Or remove that one and retain this one?
src/tests/agent_operation_feedback_tests.cpp
Lines 156 (patched)
<https://reviews.apache.org/r/69910/#comment298455>
Is this necessary?
- Greg Mann
On Feb. 6, 2019, 9:02 p.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69910/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2019, 9:02 p.m.)
>
>
> Review request for mesos, Greg Mann and Joseph Wu.
>
>
> Bugs: MESOS-9473
> https://issues.apache.org/jira/browse/MESOS-9473
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added tests for feedback for operations on agent default resources.
>
>
> Diffs
> -----
>
> src/Makefile.am 3f7daf2cca63c1b9c9e78264f241892327741aa0
> src/tests/CMakeLists.txt 42f820715ac43dc70a776f30783d9bc078ef99a5
> src/tests/agent_operation_feedback_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69910/diff/1/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh--gtest_filter="*AgentOperationFeedbackTest*" --gtest_repeat=5000 --gtest_break_on_failure`
>
>
> Thanks,
>
> Gastón Kleiman
>
>