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/07/17 01:45:14 UTC
Review Request 12603: Fixed scheduler driver to acknowledge duplicate status
updates.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Ben Mahler.
Bugs: MESOS-551
https://issues.apache.org/jira/browse/MESOS-551
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
Diff: https://reviews.apache.org/r/12603/diff/
Testing
-------
make check (OSX and Linux)
Thanks,
Vinod Kone
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/
-----------------------------------------------------------
(Updated July 17, 2013, 6:32 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benm's and bill's.
Bugs: MESOS-551
https://issues.apache.org/jira/browse/MESOS-551
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
Diff: https://reviews.apache.org/r/12603/diff/
Testing
-------
make check (OSX and Linux)
Thanks,
Vinod Kone
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 17, 2013, 5:46 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 739
> > <https://reviews.apache.org/r/12603/diff/2/?file=322317#file322317line739>
> >
> > Consider dropping the hash set if it results in less code and risk of data drift bugs. O(n) is probably unnoticeable for n=100.
I've bumped this to 10,000. So I will leave the hash set optimization, since we would be doing this work for every update.
> On July 17, 2013, 5:46 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 735
> > <https://reviews.apache.org/r/12603/diff/2/?file=322317#file322317line735>
> >
> > s/are/were/
done
> On July 17, 2013, 5:46 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 738
> > <https://reviews.apache.org/r/12603/diff/2/?file=322317#file322317line738>
> >
> > More informative variable names would be nice, perhaps ackedUpdateUuidQueue, ackedUpdateUuids. Not sure if there's a preference for conciseness, if so that may dictate.
we definitely prefer conciseness. i could just call them 'acks' but i thought that was a bit misleading too. thoughts?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23241
-----------------------------------------------------------
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Bill Farner <bi...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23241
-----------------------------------------------------------
Ship it!
This LGTM as a best-effort dedupe.
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47136>
s/are/were/
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47137>
More informative variable names would be nice, perhaps ackedUpdateUuidQueue, ackedUpdateUuids. Not sure if there's a preference for conciseness, if so that may dictate.
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47139>
Consider dropping the hash set if it results in less code and risk of data drift bugs. O(n) is probably unnoticeable for n=100.
- Bill Farner
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Ben Mahler <be...@gmail.com>.
> On July 17, 2013, 2:58 a.m., Ben Mahler wrote:
> > src/sched/sched.cpp, line 78
> > <https://reviews.apache.org/r/12603/diff/2/?file=322317#file322317line78>
> >
> > CACHED_UPDATES_CAPACITY?
> >
> > Also, maybe add a note that this only works for smaller clusters. If there are more than 100 offers in 10 seconds, this will not eliminate any duplicates and will only be adding unnecessary overhead, right?
> >
> > Caching O(20-30) seconds of updates seems nicer, but we'd have to be careful of the memory / performance overhead. Thoughts?
>
> Vinod Kone wrote:
> Changed it to "MAX_ACKED_UPDATES". I think thats more descriptive.
>
> Regarding the number, it was definitely pulled out of thin air. Thanks to your nudge, here are my thoughts.
>
> As you alluded, the critical number here is the "number of unique status updates" / "status update retry interval" generated by the cluster. Currently, the retry interval is set to 10s. Note that if the rate of status unique updates is high, the changes in this patch are irrelevant. The scheduler would still get backed up processing the updates.
>
> Now regarding MAX_ACKED_UPDATES value, the higher the number the more likely the driver can bypass the scheduler for duplicates. As far as I can tell, the only concern would be the memory consumed. Back of envelope math suggests it is roughly, 2 * 16 bytes (UUID size) * MAX_ACKED_UPDATES. For MAX_ACKED_UPDATES=1000, this value is ~32KB whereas for 10,000 it is ~320KB. IMO, ~320KB overhead for the driver is reasonable, so I will bump this number to 10,000.
>
> Thoughts?
That's a much better description that what I said! Care to add a comment reflecting this (the part about the status update rate) in the code for posterity?
SGTM, I'm ok caching O(1000-10,000) updates. This is useful for calculating the memory overhead: http://www.boost.org/doc/libs/1_54_0/doc/html/intrusive/unordered_set_unordered_multiset.html#intrusive.unordered_set_unordered_multiset.unordered_set_unordered_multiset_performance
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23230
-----------------------------------------------------------
On July 17, 2013, 6:32 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 6:32 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 17, 2013, 2:58 a.m., Ben Mahler wrote:
> > src/sched/sched.cpp, line 78
> > <https://reviews.apache.org/r/12603/diff/2/?file=322317#file322317line78>
> >
> > CACHED_UPDATES_CAPACITY?
> >
> > Also, maybe add a note that this only works for smaller clusters. If there are more than 100 offers in 10 seconds, this will not eliminate any duplicates and will only be adding unnecessary overhead, right?
> >
> > Caching O(20-30) seconds of updates seems nicer, but we'd have to be careful of the memory / performance overhead. Thoughts?
Changed it to "MAX_ACKED_UPDATES". I think thats more descriptive.
Regarding the number, it was definitely pulled out of thin air. Thanks to your nudge, here are my thoughts.
As you alluded, the critical number here is the "number of unique status updates" / "status update retry interval" generated by the cluster. Currently, the retry interval is set to 10s. Note that if the rate of status unique updates is high, the changes in this patch are irrelevant. The scheduler would still get backed up processing the updates.
Now regarding MAX_ACKED_UPDATES value, the higher the number the more likely the driver can bypass the scheduler for duplicates. As far as I can tell, the only concern would be the memory consumed. Back of envelope math suggests it is roughly, 2 * 16 bytes (UUID size) * MAX_ACKED_UPDATES. For MAX_ACKED_UPDATES=1000, this value is ~32KB whereas for 10,000 it is ~320KB. IMO, ~320KB overhead for the driver is reasonable, so I will bump this number to 10,000.
Thoughts?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23230
-----------------------------------------------------------
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23230
-----------------------------------------------------------
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47118>
CACHED_UPDATES_CAPACITY?
Also, maybe add a note that this only works for smaller clusters. If there are more than 100 offers in 10 seconds, this will not eliminate any duplicates and will only be adding unnecessary overhead, right?
Caching O(20-30) seconds of updates seems nicer, but we'd have to be careful of the memory / performance overhead. Thoughts?
- Ben Mahler
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/
-----------------------------------------------------------
(Updated July 17, 2013, 1:34 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benm's and bill's.
Summary (updated)
-----------------
Fixed scheduler driver to minimize sending duplicate status updates to the scheduler.
Bugs: MESOS-551
https://issues.apache.org/jira/browse/MESOS-551
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
Diff: https://reviews.apache.org/r/12603/diff/
Testing
-------
make check (OSX and Linux)
Thanks,
Vinod Kone
Re: Review Request 12603: Fixed scheduler driver to acknowledge duplicate
status updates.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 390
> > <https://reviews.apache.org/r/12603/diff/1/?file=322203#file322203line390>
> >
> > I'm ignorant to the implications of this, but can you confirm/deny the following behavior?
> >
> > - Queue holds [U1, U2, U3, U4] which have yet to be processed.
> >
> > - Update U1 arrives, this code processes it.
> >
> > - Scheduler aborts.
> >
> > - New scheduler receives retried [U1, U2, U2, U4] (in any order)
Not sure which queue you are referring to, but I'm assuming you mean the 'uuids' set?
An update goes into 'uuids' only after it is processed (i.e., Scheduler::statusUpdate() returns) by the scheduler.
In the above scenario if a duplicate U1 is enqueued in the libprocess queue and the scheduler aborts after handling the original U1, the driver would've aborted and we would have never come here.
When a new scheduler (and driver) becomes the leader they get updates fresh from mesos.
Does that make sense?
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/tests/status_update_manager_tests.cpp, line 469
> > <https://reviews.apache.org/r/12603/diff/1/?file=322204#file322204line469>
> >
> > From the description of this test case, it seems the desired behavior was already expected. Was there a regression at some point?
> >
> > Also, i notice that it explicitly refers to "terminal status updates". Is that still accurate, or are all equivalent updates now de-duplicated?
This test previously expected the mock scheduler to get the retried update. The update was retried by the slave, because the acknowledgement by the scheduler was dropped by the test!
Since the driver now acks it, the scheduler shouldn't receive it. Hence the need for the test fix.
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 386
> > <https://reviews.apache.org/r/12603/diff/1/?file=322203#file322203line386>
> >
> > s/,//
done
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23212
-----------------------------------------------------------
On July 16, 2013, 11:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 16, 2013, 11:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Bill Farner <bi...@twitter.com>.
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 390
> > <https://reviews.apache.org/r/12603/diff/1/?file=322203#file322203line390>
> >
> > I'm ignorant to the implications of this, but can you confirm/deny the following behavior?
> >
> > - Queue holds [U1, U2, U3, U4] which have yet to be processed.
> >
> > - Update U1 arrives, this code processes it.
> >
> > - Scheduler aborts.
> >
> > - New scheduler receives retried [U1, U2, U2, U4] (in any order)
>
> Vinod Kone wrote:
> Not sure which queue you are referring to, but I'm assuming you mean the 'uuids' set?
>
> An update goes into 'uuids' only after it is processed (i.e., Scheduler::statusUpdate() returns) by the scheduler.
>
> In the above scenario if a duplicate U1 is enqueued in the libprocess queue and the scheduler aborts after handling the original U1, the driver would've aborted and we would have never come here.
>
> When a new scheduler (and driver) becomes the leader they get updates fresh from mesos.
>
> Does that make sense?
>
> Bill Farner wrote:
> I think you explained behavior for a slightly different scenario than what i'm attempting to describe.
>
> - The driver has received [U1, U2, U3, U4], but the scheduler implementation has yet to receive/ACK them.
>
> - A duplicate U1 arrives.
>
> - Scheduler aborts.
>
> What happens in that scenario? Based on the verbiage in the diff, it sounds as though U1 is ACKed to other parts of the system, and will not be retried when the new scheduler takes over.
>
> Vinod Kone wrote:
> It is not possible for U1,U2,U3 and U4 to have been processed by the driver while the scheduler has not yet processed U1. The scheduler should've processed U1 before U2 can be processed by the driver, since it is a synchronous call into the scheduler. Subsequently, if a duplicate U1 (i.e., U1 is in 'uuids') is being processed by the driver, it means the driver has not aborted when it dealt with the original U1. Because if the driver aborted while handling the original U1, 'aborted' flag would've been set before the driver processes any other updates. Makes sense?
>
> I now realize that my comments in the code didn't justify the subtlety of the semantics. Happy to expand the comments once you are satisfied with the correctness.
>
>
Thanks for the detail. This and offline conversation helped clarify. I think i can do a more competent review now :-)
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23212
-----------------------------------------------------------
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Vinod Kone <vi...@gmail.com>.
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 390
> > <https://reviews.apache.org/r/12603/diff/1/?file=322203#file322203line390>
> >
> > I'm ignorant to the implications of this, but can you confirm/deny the following behavior?
> >
> > - Queue holds [U1, U2, U3, U4] which have yet to be processed.
> >
> > - Update U1 arrives, this code processes it.
> >
> > - Scheduler aborts.
> >
> > - New scheduler receives retried [U1, U2, U2, U4] (in any order)
>
> Vinod Kone wrote:
> Not sure which queue you are referring to, but I'm assuming you mean the 'uuids' set?
>
> An update goes into 'uuids' only after it is processed (i.e., Scheduler::statusUpdate() returns) by the scheduler.
>
> In the above scenario if a duplicate U1 is enqueued in the libprocess queue and the scheduler aborts after handling the original U1, the driver would've aborted and we would have never come here.
>
> When a new scheduler (and driver) becomes the leader they get updates fresh from mesos.
>
> Does that make sense?
>
> Bill Farner wrote:
> I think you explained behavior for a slightly different scenario than what i'm attempting to describe.
>
> - The driver has received [U1, U2, U3, U4], but the scheduler implementation has yet to receive/ACK them.
>
> - A duplicate U1 arrives.
>
> - Scheduler aborts.
>
> What happens in that scenario? Based on the verbiage in the diff, it sounds as though U1 is ACKed to other parts of the system, and will not be retried when the new scheduler takes over.
It is not possible for U1,U2,U3 and U4 to have been processed by the driver while the scheduler has not yet processed U1. The scheduler should've processed U1 before U2 can be processed by the driver, since it is a synchronous call into the scheduler. Subsequently, if a duplicate U1 (i.e., U1 is in 'uuids') is being processed by the driver, it means the driver has not aborted when it dealt with the original U1. Because if the driver aborted while handling the original U1, 'aborted' flag would've been set before the driver processes any other updates. Makes sense?
I now realize that my comments in the code didn't justify the subtlety of the semantics. Happy to expand the comments once you are satisfied with the correctness.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23212
-----------------------------------------------------------
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to minimize sending
duplicate status updates to the scheduler.
Posted by Bill Farner <bi...@twitter.com>.
> On July 17, 2013, 12:15 a.m., Bill Farner wrote:
> > src/sched/sched.cpp, line 390
> > <https://reviews.apache.org/r/12603/diff/1/?file=322203#file322203line390>
> >
> > I'm ignorant to the implications of this, but can you confirm/deny the following behavior?
> >
> > - Queue holds [U1, U2, U3, U4] which have yet to be processed.
> >
> > - Update U1 arrives, this code processes it.
> >
> > - Scheduler aborts.
> >
> > - New scheduler receives retried [U1, U2, U2, U4] (in any order)
>
> Vinod Kone wrote:
> Not sure which queue you are referring to, but I'm assuming you mean the 'uuids' set?
>
> An update goes into 'uuids' only after it is processed (i.e., Scheduler::statusUpdate() returns) by the scheduler.
>
> In the above scenario if a duplicate U1 is enqueued in the libprocess queue and the scheduler aborts after handling the original U1, the driver would've aborted and we would have never come here.
>
> When a new scheduler (and driver) becomes the leader they get updates fresh from mesos.
>
> Does that make sense?
I think you explained behavior for a slightly different scenario than what i'm attempting to describe.
- The driver has received [U1, U2, U3, U4], but the scheduler implementation has yet to receive/ACK them.
- A duplicate U1 arrives.
- Scheduler aborts.
What happens in that scenario? Based on the verbiage in the diff, it sounds as though U1 is ACKed to other parts of the system, and will not be retried when the new scheduler takes over.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23212
-----------------------------------------------------------
On July 17, 2013, 1:34 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 17, 2013, 1:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to acknowledge duplicate
status updates.
Posted by Bill Farner <bi...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23212
-----------------------------------------------------------
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47077>
s/,//
src/sched/sched.cpp
<https://reviews.apache.org/r/12603/#comment47076>
I'm ignorant to the implications of this, but can you confirm/deny the following behavior?
- Queue holds [U1, U2, U3, U4] which have yet to be processed.
- Update U1 arrives, this code processes it.
- Scheduler aborts.
- New scheduler receives retried [U1, U2, U2, U4] (in any order)
src/tests/status_update_manager_tests.cpp
<https://reviews.apache.org/r/12603/#comment47078>
From the description of this test case, it seems the desired behavior was already expected. Was there a regression at some point?
Also, i notice that it explicitly refers to "terminal status updates". Is that still accurate, or are all equivalent updates now de-duplicated?
- Bill Farner
On July 16, 2013, 11:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 16, 2013, 11:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 12603: Fixed scheduler driver to acknowledge duplicate
status updates.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12603/#review23216
-----------------------------------------------------------
Can you update the subject to reflect that this is more of an "optimization" than a "fix"?
- Ben Mahler
On July 16, 2013, 11:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12603/
> -----------------------------------------------------------
>
> (Updated July 16, 2013, 11:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-551
> https://issues.apache.org/jira/browse/MESOS-551
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 7ea82e547c612159c9fa24fb6d62e3d2b5f11982
> src/tests/status_update_manager_tests.cpp 42395324dfe49659bee2229c6573ffef0874d923
>
> Diff: https://reviews.apache.org/r/12603/diff/
>
>
> Testing
> -------
>
> make check (OSX and Linux)
>
>
> Thanks,
>
> Vinod Kone
>
>