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/03/26 21:54:26 UTC
Review Request: Fixed status update manager to create the updates path by
itself instead of passing the path from the slave.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Ben Mahler.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 091ec5ed19924aef31b761e68b70b8d042f9a9b7
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/utils.hpp 32784a79804ecb8758fb9ae6cdb4bb1e87c29252
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19274
-----------------------------------------------------------
Ship it!
src/slave/slave.hpp
<https://reviews.apache.org/r/10142/#comment39921>
Point out that this implies the framework is shutting down everywhere.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39919>
Log the fact of whether we could remove it from the queued tasks for the executor.
If we can't then the WARNING should be logged as before.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39920>
WARNING?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39922>
s/This/TERMINATED/
s/double// and "runs the executor driver in a child process"
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39923>
Ditto
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39925>
Comment as to how this state is possible.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39926>
Add an explicit case for TERMINATED that falls through to default.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39927>
Can you remove this for me?
Ditto for _unwatch :)
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39928>
This needs to check discarded if we remove the CHECK.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39931>
model this after the switch style in json.hpp
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39930>
kill breaks in these
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39929>
either default or this, but not both
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39932>
ditto
- Ben Mahler
On April 15, 2013, 11:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 15, 2013, 11:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
> src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 17, 2013, 6:38 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
comments and rebase. no need for review.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/isolator.hpp 7c9a095441baf28c99f0cdef3a005f0a9944ee78
src/tests/slave_recovery_tests.cpp 7be332b74367869e2b4c847fea348535cc526a23
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 15, 2013, 11:54 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benm's comments.
also overloaded ostream operators for states.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Ben Mahler <be...@gmail.com>.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > 1. All of the CHECK statements need to output the state. This could be obviated by just using switches / ifs everywhere.
> >
> > 2. I like the clarity of the switch statements, but they do have the downside that you sometimes use if statements, and sometimes use switch statements. I'm guessing this was to avoid nesting. Just a note, I'm good with the switches.
3. Use explicit case statements for all the states as discussed offline.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19115
-----------------------------------------------------------
On April 12, 2013, 9:24 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 12, 2013, 9:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
> src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Vinod Kone <vi...@gmail.com>.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.hpp, line 391
> > <https://reviews.apache.org/r/10142/diff/7/?file=280869#file280869line391>
> >
> > If present tense above, then present tense here as well.
switched to present tense.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1139
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1139>
> >
> > Why?
If the parent dies and the child registers, we end up in this situation. Beefed up the comment, here and in reregisterExecutor().
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1261
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1261>
> >
> > Signal to whom?
killed the signaling bit. we no longer use the pid to signify registration. we use the executor state instead.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1375
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1375>
> >
> > newline?
This code changed in a subsequent review, so this is N/A. I will avoid touching it here, to make the rebase easy.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1382
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1382>
> >
> > Can you either change this to an else if RUNNING or do a CHECK(RUNNING) on the framework state?
I will fix this in the downstream review that changes this logic. Thanks for catching.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1384
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1384>
> >
> > newline?
see above.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1393
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1393>
> >
> > Ditto for the executor state here. Either if or CHECK for RUNNING, or all the expected states.
see above.
> On April 12, 2013, 11:51 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1824
> > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1824>
> >
> > Do you want to CHECK the expected states?
Again. Dropping this in favor of fixing it in the review that refactors this code.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19115
-----------------------------------------------------------
On April 12, 2013, 9:24 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 12, 2013, 9:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
> src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19115
-----------------------------------------------------------
1. All of the CHECK statements need to output the state. This could be obviated by just using switches / ifs everywhere.
2. I like the clarity of the switch statements, but they do have the downside that you sometimes use if statements, and sometimes use switch statements. I'm guessing this was to avoid nesting. Just a note, I'm good with the switches.
src/slave/slave.hpp
<https://reviews.apache.org/r/10142/#comment39670>
Use either present or past tense in all of these? I prefer present tense:
// Executor is launched, but not (re-)registered yet.
...
src/slave/slave.hpp
<https://reviews.apache.org/r/10142/#comment39671>
If present tense above, then present tense here as well.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39672>
Why?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39673>
Signal to whom?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39674>
newline?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39675>
Can you either change this to an else if RUNNING or do a CHECK(RUNNING) on the framework state?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39676>
newline?
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39677>
Ditto for the executor state here. Either if or CHECK for RUNNING, or all the expected states.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39678>
Do you want to CHECK the expected states?
- Ben Mahler
On April 12, 2013, 9:24 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 12, 2013, 9:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
> src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 12, 2013, 9:24 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off trunk. No Need For Review (NNFR).
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed slave to explicitly check for framework and
executor states.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 12, 2013, 7:17 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Tested against master tests.
Summary (updated)
-----------------
Fixed slave to explicitly check for framework and executor states.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
> On April 11, 2013, 8:52 p.m., Vinod Kone wrote:
> >
After talking to BenM, I've decided to use switch statements for state checking. I think this makes it much easier to reason about the states of frameworks and executors.
Let me know how it looks!
> On April 11, 2013, 8:52 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 1003
> > <https://reviews.apache.org/r/10142/diff/5/?file=279791#file279791line1003>
> >
> > checks
not needed here.
> On April 11, 2013, 8:52 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, lines 1219-1222
> > <https://reviews.apache.org/r/10142/diff/5/?file=279791#file279791line1219>
> >
> > Verify and change the semantics of how we do SUM.update() w/ and w/o checkpoint.
I'm going to split the SUM related changes into its own review.
> On April 11, 2013, 8:52 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 1197
> > <https://reviews.apache.org/r/10142/diff/5/?file=279791#file279791line1197>
> >
> > check for non null pid
actually, since we recover the pid we can't check for non-existence of pid here. this was an existing bug!
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19027
-----------------------------------------------------------
On April 9, 2013, 8:58 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 8:58 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/#review19027
-----------------------------------------------------------
src/slave/slave.hpp
<https://reviews.apache.org/r/10142/#comment39549>
kill unused states.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39538>
Comment
s/Not killing/Ignoring/
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39539>
Remove the task from executor's queued tasks
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39540>
Ignoring
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39541>
add a CHECK(RUNNING) here
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39542>
checks
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39543>
checks
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39544>
check for non null pid
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39546>
Verify and change the semantics of how we do SUM.update() w/ and w/o checkpoint.
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39547>
check states..here and everywhere else
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39548>
s/TERMINATED/TERMINATING/
src/slave/slave.cpp
<https://reviews.apache.org/r/10142/#comment39550>
Change this to CHECK(!terminated)
- Vinod Kone
On April 9, 2013, 8:58 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10142/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 8:58 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> This is based off https://reviews.apache.org/r/10112.
>
> Also fixed TODOs and other misc stuff from the above review.
>
>
> Diffs
> -----
>
> src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
> src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
> src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
> src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
> src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
> src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
>
> Diff: https://reviews.apache.org/r/10142/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 9, 2013, 8:58 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off trunk.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 2, 2013, 7:41 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off latest trunk.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/utils.hpp 32784a79804ecb8758fb9ae6cdb4bb1e87c29252
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated April 2, 2013, 1:58 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Jiang Yan Xu.
Changes
-------
statusUpdate() now rejects updates when framework is terminating.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/utils.hpp 32784a79804ecb8758fb9ae6cdb4bb1e87c29252
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated March 29, 2013, 9:47 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off trunk.
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs (updated)
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/utils.hpp 32784a79804ecb8758fb9ae6cdb4bb1e87c29252
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request: Fixed status update manager to create the updates path
by itself instead of passing the path from the slave.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10142/
-----------------------------------------------------------
(Updated March 26, 2013, 10:18 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
no-op
Description
-------
This is based off https://reviews.apache.org/r/10112.
Also fixed TODOs and other misc stuff from the above review.
Diffs
-----
src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741
src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0
src/slave/slave.cpp 091ec5ed19924aef31b761e68b70b8d042f9a9b7
src/slave/status_update_manager.hpp e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24
src/slave/status_update_manager.cpp 044d245f370ef23ddc67fadbf7f8fe9d75dd662a
src/tests/utils.hpp 32784a79804ecb8758fb9ae6cdb4bb1e87c29252
Diff: https://reviews.apache.org/r/10142/diff/
Testing
-------
make check
Thanks,
Vinod Kone