You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/05/30 23:59:23 UTC
Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Description
-------
See summary.
This addresses bug MESOS-482.
https://issues.apache.org/jira/browse/MESOS-482
Diffs
-----
src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
Diff: https://reviews.apache.org/r/11549/diff/
Testing
-------
make check
GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21325
-----------------------------------------------------------
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44231>
Great!
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44232>
You won't get this yet, please move down.
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44233>
EXPECT_CALL(sched, statusUpdate ...) should come here.
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44234>
Please add a comment explaining this set up for the rest of the test.
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44235>
So can 'update' and 'update2' be moved down here? Does Clock::advance act as the event which sends those two updates? If not, can you move one of the AWAIT_READY(update*) calls up?
- Benjamin Hindman
On May 30, 2013, 9:59 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated May 30, 2013, 9:59 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Vinod Kone <vi...@gmail.com>.
> On June 3, 2013, 11:35 p.m., Ben Mahler wrote:
> > What was the motivation for this? This seems to allow multiple terminal updates, which seems conflicting, no?
We had to do this because some frameworks use terminal updates to do task reconciliation. Added a TODO to disallow it once we have all the required pieces in place (e.g., registrar).
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21368
-----------------------------------------------------------
On June 4, 2013, 1:02 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated June 4, 2013, 1:02 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Ben Mahler <be...@gmail.com>.
> On June 3, 2013, 11:35 p.m., Ben Mahler wrote:
> > What was the motivation for this? This seems to allow multiple terminal updates, which seems conflicting, no?
>
> Vinod Kone wrote:
> We had to do this because some frameworks use terminal updates to do task reconciliation. Added a TODO to disallow it once we have all the required pieces in place (e.g., registrar).
Ah, I see!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21368
-----------------------------------------------------------
On June 4, 2013, 1:02 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated June 4, 2013, 1:02 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21368
-----------------------------------------------------------
What was the motivation for this? This seems to allow multiple terminal updates, which seems conflicting, no?
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44304>
s/a/another/
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44303>
s/acknowledges/acknowledge/
- Ben Mahler
On June 3, 2013, 10:30 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:30 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21504
-----------------------------------------------------------
Ship it!
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/11549/#comment44509>
I think this needs more comments explaining why you are injecting another status update.
- Benjamin Hindman
On June 4, 2013, 1:02 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated June 4, 2013, 1:02 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/#review21402
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On June 4, 2013, 1:02 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11549/
> -----------------------------------------------------------
>
> (Updated June 4, 2013, 1:02 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
>
> This addresses bug MESOS-482.
> https://issues.apache.org/jira/browse/MESOS-482
>
>
> Diffs
> -----
>
> src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
> src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
> src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
>
> Diff: https://reviews.apache.org/r/11549/diff/
>
>
> Testing
> -------
>
> make check
>
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/
-----------------------------------------------------------
(Updated June 4, 2013, 1:02 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
benm's.
Description
-------
See summary.
This addresses bug MESOS-482.
https://issues.apache.org/jira/browse/MESOS-482
Diffs (updated)
-----
src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
Diff: https://reviews.apache.org/r/11549/diff/
Testing
-------
make check
GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to terminate the stream only
when it has no pending updates.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11549/
-----------------------------------------------------------
(Updated June 3, 2013, 10:30 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
benh's.
Description
-------
See summary.
This addresses bug MESOS-482.
https://issues.apache.org/jira/browse/MESOS-482
Diffs (updated)
-----
src/slave/status_update_manager.hpp 795e74c2b88a071eb7ba720118e06077b6e41238
src/slave/status_update_manager.cpp 9e9e4e2a47a609d65ed69a57de595852144a86c8
src/tests/status_update_manager_tests.cpp 61ccfcc8165337d02af5641884beee45a2aaa065
Diff: https://reviews.apache.org/r/11549/diff/
Testing
-------
make check
GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TerminateEmptyStream*" --verbose --gtest_repeat=1000 --gtest_break_on_failure
Thanks,
Vinod Kone